Conversation
Motivation: HttpServerCodec always constructs an `EmptyLastHttpContent` and passes it to the Handler chain when processing OPTIONS requests, The `EmptyLastHttpContent` propagate through the handler chain. However, the CORS handler might still propagate the EmptyLastHttpContent to downstream handlers via fireChannelRead(), causing subsequent handlers to receive only this empty content and lose access to the original request URL. Because `CorsHandler` does not consume this message, it calls `ctx.fireChannelRead(msg)` for the EmptyLastHttpContent. Downstream handlers then observe: No HttpRequest A LastHttpContent with empty content This often breaks other handlers that rely on receiving the `HttpRequest` first or expect consistent HTTP message. This PR fixes an issue in `CorsHandler` where, after handling a CORS preflight (OPTIONS) request, the handler still propagates the subsequent `EmptyLastHttpContent` sent by the `HttpServerCodec` to downstream handler. Modification: - `CorsHandler` track handled preflight and consume the following HttpContent, LastHttpContent until the next HttpRequest is forwarded instead of firing it downstream. - Add tests Result: Fixes #15148 It also improves compatibility with application frameworks and routing handlers that expect well-formed HTTP request/response flows. --------- Signed-off-by: skyguard1 <qhdxssm@qq.com> Co-authored-by: xingrufei <xingrufei@yl.com> Co-authored-by: code-xing_wcar <code.xing@wirelesscar.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
HttpServerCodec always constructs an
EmptyLastHttpContentand passes it to the Handler chain when processing OPTIONS requests, TheEmptyLastHttpContentpropagate through the handler chain.However, the CORS handler might still propagate the EmptyLastHttpContent to downstream handlers via fireChannelRead(), causing subsequent handlers to receive only this empty content and lose access to the original request URL.
Because
CorsHandlerdoes not consume this message, it callsctx.fireChannelRead(msg)for the EmptyLastHttpContent.Downstream handlers then observe:
No HttpRequest
A LastHttpContent with empty content
This often breaks other handlers that rely on receiving the
HttpRequestfirst or expect consistent HTTP message.This PR fixes an issue in
CorsHandlerwhere, after handling a CORS preflight (OPTIONS) request, the handler still propagates the subsequentEmptyLastHttpContentsent by theHttpServerCodecto downstream handler.Modification:
CorsHandlertrack handled preflight and consume the following HttpContent, LastHttpContent until the next HttpRequest is forwarded instead of firing it downstream.Result:
Fixes #15148
It also improves compatibility with application frameworks and routing handlers that expect well-formed HTTP request/response flows.