Repository navigation
Conversation
ServletRequestCopy, the HttpServletRequest Wicket keeps for the lifetime of a WebSocket connection, stored the header enumerations of the request it copied and handed the same ones out on every read. It looked them up by the exact name the container reported, so a lowercase "origin", as sent over HTTP/2 and by many proxies, was not found as "Origin". Each enumeration could be read once: getHeader consumed a value, so a second call returned the next value or null; getHeaders returned the spent enumeration; getIntHeader consumed it as well and threw a NullPointerException for an absent header. getHeaders returned null for an absent header, which ServletWebRequest dereferences. getLocales had the same single-use problem. JavaxUpgradeHttpRequest, the request every JSR 356 connection is copied from, kept the handshake header map as it came, looked names up case-sensitively and returned null from getHeaders and getHeaderNames when there was nothing to return. Both now copy the headers when they are created, into a map whose names compare case-insensitively as the Servlet specification requires, and return a fresh enumeration on every read, empty rather than null when there are no values. ServletRequestCopy returns -1 from getIntHeader for an absent header and copies the locales the same way. A name the copied request reports more than once in different case is copied once, because that request's getHeaders already returns all of its values; JavaxUpgradeHttpRequest merges the values of handshake map keys that differ only in case. An application now sees a WebSocket request that answers header lookups the way the handshake request did. WebSocketConnectionOriginFilter no longer rejects a connection whose Origin header arrived in lowercase. getHeaderNames lists each name once, in case-insensitive alphabetical order. Only private fields change; there is no API change. GitHub issue #1688
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
1 participant
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.
Backport of #1689 to
wicket-10.x, for GitHub issue #1688.ServletRequestCopyandJavaxUpgradeHttpRequestlook header names up case-insensitively, return the same values on every read, and return an empty enumeration rather thannullfor an absent header. A WebSocket connection whoseOriginheader arrives in lowercase (HTTP/2, many proxies) is no longer rejected byWebSocketConnectionOriginFilter, andWebRequest#getHeaderson a WebSocket request no longer throws aNullPointerExceptionfor an absent header. Keep the Servlet header contract in the WebSocket request copies #1689 has the details.Only private fields change: japicmp has nothing to report against 10.0.0, and applications have nothing to migrate.
Verified on this branch:
ServletRequestCopyTestandJavaxUpgradeHttpRequestTest, 10 tests, pass.mvn clean verify -Pjs-testis green on JDK 17, japicmp included.This goes in after #1689. If #1689 changes in review, this branch gets the same change.