-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Improve logging and error messages when mismatch between client and server protocol/TLS #14884
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
Comments
Do you have examples for the client and server errors you see for each of these? Expect different client errors in browsers vs libraries (HttpClient).
With TLS: Without TLS:
Similar to case 1. We should already be logging a decent error for the non-TLS scenario when the preamble doesn't match, but we may not be sending it to the client.
There's no such thing as a HTTP/1 + HTTP/2 port without TLS (Upgrades asside). The two can only be combine with TLS and ALPN. In practice this is the same as case 1 above.
Would require intercepting the TLS handshake. Cost: Medium.
I thought we already added a server log for this in 3.0? Sending an error to the client here would be hard, we'd have to emulate TLS. A TLS fatal Alert maybe? |
Since the cause, solution, and priority for each of these is so different we'll want to break out any we decide to address into individual issues. |
I'll investigate. Will update this issue and then we can create more targeted issues. |
Test app: https://github.com/JamesNK/HttpConnectionErrors Server output:
|
Nice permutation. You're not logging the client errors? Next would be grouping by error message. |
I'm doing the same exercise with HttpClient on dotnet/corefx 😄 Client errors: https://github.com/dotnet/corefx/issues/41701#issuecomment-540777343 |
Grouped: Client: HTTP/1.1 - No TLS Client: HTTP/1.1 - No TLS Client: HTTP/1.1 - No TLS Client: HTTP/1.1 - No TLS Client: HTTP/1.1 - No TLS Client: HTTP/1.1 - No TLS Client: HTTP/2 - No TLS Client: HTTP/2 - No TLS Client: HTTP/2 - No TLS Client: HTTP/2 - No TLS Client: HTTP/2 - No TLS Client: HTTP/2 - No TLS
Client: HTTP/1.1 - No TLS Client: HTTP/1.1 - TLS Client: HTTP/2 - TLS
Client: HTTP/1.1 - TLS Client: HTTP/1.1 - TLS
Client: HTTP/1.1 - TLS Client: HTTP/1.1 - TLS Client: HTTP/2 - TLS Client: HTTP/2 - TLS
Client: HTTP/1.1 - TLS Client: HTTP/1.1 - TLS
Client: HTTP/2 - No TLS Client: HTTP/2 - No TLS
Client: HTTP/2 - TLS Client: HTTP/2 - TLS
Client: HTTP/2 - TLS
Client: HTTP/2 - TLS
|
Is there proposed action here? I think this seems good and I'm all for sniffing different protocol stuff to produce better errors, but we need to break it down into specific actions we want to take. |
Action: Let's build the matrix and try to produce useful errors for all the combinations in the logs at least. We may be able to do things like return 400 errors or TLS alerts, but I don't want to make the scope of this too big :). |
I posed some questions when I grouped all the errors together. The action is for a server expert to figure out whether these errors could be improved. I don't think improving them is worth spending a lot of resources on, but it would be good to figure out if there are any quick wins to improve the debugging experience. |
@JamesNK is this something that can be done by a contributor? |
HTTP/1.1 sent to HTTP/2 endpoint and vice versa is done. Remaining is detecting TLS with a non-TLS endpoint and vice versa. |
My experience with gRPC is that it is very common to run into trouble establishing a connection between the client and the server because of incorrect protocol/TLS.
The logging and error messages that Kestrel/IIS provides should do as much as possible to to help developers fix their own mistakes. Also I would like to make sure that we're sending the best response to clients in these situations to help the client give the developer a good understanding of what is wrong.
Common problem situations:
The text was updated successfully, but these errors were encountered: