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

Fix NPE when failing to decode response from server #330

Merged
merged 4 commits into from Dec 23, 2016

Conversation

jekh
Copy link
Collaborator

@jekh jekh commented Dec 19, 2016

This PR should fix issue #303. It does two things:

  • Keeps track of the current request in ProxyToServerConnection as soon as it is sent to the server. Removes the "queue" of current requests and the need to "pop" from the queue at some point while processing the response from the server. The code always assumed there would be only one request per connection, so having a stack of requests is unnecessary.
  • When netty fails to decode a response from the server, this will "substitute" a new 502 Bad Gateway response and send that to the client, closing the connection to the server in the process. This is the most sensible thing we can do, since a decoder failure means the response is too malformed to make any sense of.

The current code already assumes there is one request outstanding per connection at a time. This is sensible in HTTP 1.1 (pipelining notwithstanding, though that's rarely used), but will need to change when we support HTTP/2.

@jekh
Copy link
Collaborator Author

jekh commented Dec 23, 2016

I've been testing with this patch for the last several days and haven't seen any issues. Since it's been such a problem for a long time, I'll go ahead and merge. If anybody has any review comments/objections, I'll correct or revert as needed.

@jekh jekh merged commit 0d16280 into adamfisk:master Dec 23, 2016
@jekh jekh deleted the fix-npe-from-invalid-request branch December 23, 2016 19:34
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 this pull request may close these issues.

None yet

1 participant