Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access to HTTP request bodies #181

Closed
zaphoyd opened this issue Feb 1, 2013 · 13 comments
Closed

Access to HTTP request bodies #181

zaphoyd opened this issue Feb 1, 2013 · 13 comments
Assignees
Milestone

Comments

@zaphoyd
Copy link
Owner

zaphoyd commented Feb 1, 2013

Feature request to allow WebSocket++ to handle POST/PUT/ other non-GET methods with request bodies.

Example usage case: File Uploads (#164)

This is low priority because WebSocket++ is primarily a WebSocket server and not an HTTP server. However, the request is not particularly hard and WebSocket++ can already be hacked a bit to do it so a more clean/stable interface might be useful.

@nokome
Copy link

nokome commented Nov 17, 2014

+1

zaphoyd added a commit that referenced this issue Nov 19, 2014
Use connection::get_request_body() to access. Use
endpoint::set_max_http_body_size(size_t value) to control maximum
upload size. Initial support is for single chunk bodies that define a
content-length . Transfer-Encoding: chunked is not currently supported
@zaphoyd
Copy link
Owner Author

zaphoyd commented Nov 19, 2014

First stab at support for HTTP request bodies is available on the develop branch.

Use connection::get_request_body() to access. Use endpoint::set_max_http_body_size(size_t value) to control maximum upload size. Initial support is for single chunk bodies that define a content-length header.

Question for those interested in this topic: How important is support for Transfer-Encoding: chunked?

@nokome
Copy link

nokome commented Nov 19, 2014

Thanks for your work on this. For me, Transfer-Encoding: chunked is not that important.

@zaphoyd zaphoyd added this to the 0.5.0 milestone Nov 19, 2014
@zaphoyd zaphoyd self-assigned this Nov 19, 2014
@pettno
Copy link

pettno commented Nov 26, 2014

Thanks for this! Chunked is not important for us.

Without the con->get_socket().close(); in #164, the connection sends this before closing:

HTTP/1.1 500 Internal Server Error
Server: WebSocket++/0.4.x-dev

Feature or bug? The connection has not been upgraded to a websocket.

@zaphoyd
Copy link
Owner Author

zaphoyd commented Nov 26, 2014

WebSocket++ may return a 500 error if your http handler doesn't provide a more specific one. If you are calling set_status with another code then there may be a bug here. Example:

con->set_status(websocketpp::http::status_code::ok);

@pettno
Copy link

pettno commented Nov 26, 2014

Replacing get_socket().close() with set_status(websocketpp::http::status_code::ok) removed the unwanted second http header. Thanks.

Log output due to the on_http() is:

[2014-11-26 13:56:03] [info] asio async_write error: system:9 (Bad file descriptor)
[2014-11-26 13:56:03] [error] handle_send_http_response error: websocketpp.transport:2 (Underlying Transport Error)
[2014-11-26 13:56:03] [info] asio async_shutdown error: system:9 (Bad file descriptor)

@zaphoyd
Copy link
Owner Author

zaphoyd commented Dec 1, 2014

has anyone who has been using http request body support ran into issues with hitting the handshake/request open timeouts? I believe, at present, the reading of body payload will fall within the handshake/header timer. I think it might make sense to stop this timer after the headers of an HTTP request are completed but am interested in more feedback on what sort of limits & anti-DoS features folks who are needing the payloads are interested in.

@paresy
Copy link

paresy commented Dec 4, 2014

Yes. This is exactly the problem i am experiencing.

I would stop the old timers and start another timer if the user wishes to.
By default i would set this option to zero (which means disabled and should wait forever for long uploads)

@pettno
Copy link

pettno commented Dec 15, 2014

Both set_status(ok) and get_socket.close() seems to be required to always avoid the unwanted second http ok headers. My previous comment incorrectly stated "replacing". I actually added the set_status. The error logs looks rather suspicious.

@zaphoyd
Copy link
Owner Author

zaphoyd commented Dec 15, 2014

Get_socket.close should never be required, can you post an example http handler that causes this?

@pettno
Copy link

pettno commented Dec 16, 2014

I guess I am trying to do a bit more. From issue #266, with usage of the new get_request_body():

void OurServerThing::on_http( websocketpp::connection_hdl Handler )
{
    // HTTP proxy to another web server
    using namespace std;
    using namespace boost;
    using namespace boost::asio::ip;
    try {
        char Buffer[10240];
        server::connection_ptr Connection = _WsppServer.get_con_from_hdl( Handler );
        tcp::socket & ServerSocket = Connection->get_socket();

        // Connect the HTTP client
        asio::io_service IoService;
        tcp::resolver Resolver( IoService );
        tcp::resolver::query Query( tcp::v4(), "127.0.0.1", "8080" );
        tcp::resolver::iterator Iterator = Resolver.resolve( Query );
        tcp::socket ClientSocket( IoService );
        asio::connect( ClientSocket, Iterator );

        // Proxy the WebSocket++ server request to the client
        string Request = Connection->get_request().raw();
        Request += Connection->get_request_body();
        ClientSocket.write_some( asio::buffer( Request, Request.length() ) );

        // Read and write until an error occurs
        size_t Length;
        boost::system::error_code Error;
        do {
            Length = ClientSocket.read_some( asio::buffer( Buffer, sizeof(Buffer) ), Error );
            if( !Error ) {
                ServerSocket.write_some( asio::buffer( Buffer, Length ), Error );
            }
        } while( !Error );

        // Update WebSocket++ internal status; this seems rather pointless
        Connection->set_status( websocketpp::http::status_code::ok );

        // Headers have already been read from the client and sent to the server above.
        // Close the socket now to avoid prepended HTTP headers from WebSocket++
        ServerSocket.close();
    }
    catch( websocketpp::lib::error_code e ) {
        cerr << "Exception: " << e.message() << endl;
    }
    catch( std::exception& e ) {
        cerr << "Exception: " << e.what() << endl;
    }
}

@zaphoyd
Copy link
Owner Author

zaphoyd commented Dec 16, 2014

alright. This is non-standard usage, which is why the library is printing confused looking error logs. The situation:

WebSocket++ expects to write the HTTP response to an HTTP request. By calling set_header, set_status, set_body, etc you are building up an internal request object that will be written back to the socket after the http handler ends.

In your case you want to essentially transfer ownership & control of the underlying socket and complete the request manually. In that case, when your http handler returns, if you leave the socket open, WebSocket++ will continue to think it is supposed to send the HTTP response. This results in the second response that you are seeing on the wire. Closing the socket suppresses this, as WebSocket++'s response write now fails, which the library isn't expecting, and hence why you get alarmed error messages.

Ultimately the best solution here is proper proxy support in the library (#266). In the meantime, and to add more general flexibility for cases like this, I've adding an additional check after the HTTP handler is run to detect cases like this.

Usage for the mode where your http handler wants to manually write its own response:

void OurServerThing::on_http( websocketpp::connection_hdl Handler )
    server::connection_ptr Connection = _WsppServer.get_con_from_hdl( Handler );
    tcp::socket & ServerSocket = Connection->get_socket();
    // write to ServerSocket here
    Connection->terminate(websocketpp::error::make_error_code(websocketpp::error::http_connection_ended));
}

This will invoke the WebSocket++ connection termination routine that will cleanly expire timers, close sockets, write any necessary logs, etc. Using the http_connection_ended status code will let the library know what is going on so it won't freak out and write a lot of spurious error messages. You should get a single log message on the HTTP channel saying that the HTTP connection was taken over by the handler. If using connection::terminate there is no need to call set_status, as WebSocket++ will never send that response. You also wont need to close the server socket, as terminate will do that. You'll be responsible for cleaning up any of the client sockets and such.

A few other notes:

  • request::raw() should return the request with the body included. I see you appending the body again. Is raw() not working as intended here?
  • As of 0.4.0 WebSocket++ throws exceptions of type websocketpp::exception (which derives from std::exception). Catching exceptions of type websocketpp::lib::error_code is no longer necessary, the catch (std::exception) should get them all

@pettno
Copy link

pettno commented Dec 16, 2014

Thanks for the helpful notes and changes. This helped a lot.

@zaphoyd zaphoyd closed this as completed Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants