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

Do less HTTP in test client.c/server.c examples #496

Open
cpu opened this issue Dec 9, 2024 · 0 comments · May be fixed by #497
Open

Do less HTTP in test client.c/server.c examples #496

cpu opened this issue Dec 9, 2024 · 0 comments · May be fixed by #497

Comments

@cpu
Copy link
Member

cpu commented Dec 9, 2024

When the client.c example makes a GET request it always sends a Connection: close header:

"Connection: close\r\n"

However, the server.c example does no processing of this header, and instead leaves the connection open as long as the client does.

This means that the client.c example spends a lot of code trying to process the HTTP response to find a Content-Length header it can use to determine when its read enough plaintext data to close the connection itself. As noted in #485 this doesn't work at all for servers that use chunked encoding, where no Content-Length header is expected to be present.

In practice HTTP 1.1 is very complex and C is a cumbersome language to use to implement an HTTP client from scratch. I believe we should try and reduce this complexity as much as possible and focus exclusively on demonstrating using rustls-ffi as a simple TLS client. E.g. our example should be closer to tlsclient-mio.rs from the upstream Rustls repo's examples. It does no processing of the response and instead just echoes the read plaintext to stdout and closes the connection when no more data is available.

The simplest way to do this is:

  • Change the server.c example to close the connection after its written a full response. This is technically in contradiction to HTTP1.1's implied default of Connection: keep-alive, but so is ignoring a client's Connection: close 🤪 so we're no worse off but can at least simplify client.c as a result. It doesn't seem interesting to our audience to try and implement this properly at the expense of a lot of brittle C code.
  • Change the client.c example to not attempt to process the HTTP response, and instead just read as much plaintext as possible, writing it to stdout. This will be sufficient to keep our tests between client/server working nicely and will side-step the need to understand chunked encoding for other servers that use it, we can just dump whatever we get.
  • Change the client.c to use a timeout on select(). The server example already uses a timeout for its equivalent select() but the client example does not. This means if we connect to a server that ignores Connection: close (e.g. like the existing server.c code) the client will hang forever waiting for more data that will never come (because the server is waiting for the client to issue another request or close the conn and we will do neither). To fix this, we can add a timeout. If the client waits longer for the timeout we can close the connection and treat it as an error. The client example is still "vulnerable" to a server that dribbles out little bits of data forever but these examples shouldn't be viewed as battle-hardened HTTP clients. Use libcurl for that, please.

In sum I think these changes will make the examples easier to follow and will allow reliable operation against more servers in a demo/testing context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant