Repository navigation
Disable cookie handling by default for the Gateway Server MVC HTTP client - #4352
Open
Asyncronize wants to merge 1 commit into
Open
Asyncronize wants to merge 1 commit into
Asyncronize wants to merge 1 commit into
Conversation
…ient Apache HttpComponents and Jetty keep a cookie store per client, and the proxy client is shared by every caller, so a Set-Cookie from one proxied response was sent with later requests from other callers. Default spring.http.clients.cookie-handling to disable unless it is set, the same way spring.http.clients.redirects already defaults to dont-follow. Fixes spring-cloudgh-3311 Signed-off-by: Asyncronize <56194330+Asyncronize@users.noreply.github.com>
This branch has not been deployed
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
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.
Follow-up to the plan in #3311 (thanks @spencergibb for the go-ahead).
Apache HttpComponents and Jetty both keep a cookie store by default, and the HTTP client the gateway uses to proxy requests is shared by every caller. So a
Set-Cookiefrom one proxied response gets stored and sent with later requests to the backend, including requests from other users. The JDK client, Reactor Netty and the simple factory don't store cookies, so they weren't affected.This sets
spring.http.clients.cookie-handlingtodisableinGatewayHttpClientEnvironmentPostProcessorwhen the user hasn't set it, the same wayspring.http.clients.redirectsalready defaults todont-follow. Both defaults now go into the onegatewayHttpClientPropertiesproperty source, since adding a second source with the same name would replace the first.I checked how Boot applies
DISABLEfor each builder before relying on it: HttpComponents usesStandardCookieSpec.IGNORE, Jetty an empty cookie store, JDK noCookieHandler, and Reactor Netty and the simple factory only rejectENABLE. Sodisabledoesn't break any of them, which was the open question in my comment on the issue.Tests:
GatewayServerMvcAutoConfigurationTests: the default ends up inHttpClientSettings(viaSpringApplication, since the post processor doesn't run withApplicationContextRunner), a user value is kept, and only missing defaults are added.CookieHandlingTestsinintegration-tests/httpclient, which already has httpclient5: two requests through the gateway, the backend sets a cookie on the first and the test checks the second doesn't carry it, forhttp-components,reactor,jdkandsimple. On main thehttp-componentscase fails withsession=abc123.Jetty isn't on any test classpath, so I didn't add a case for it here. I ran the same test locally with
jetty-clientadded and it behaves like HttpComponents (cookie sent on main, not with this change). Happy to add the dependency if you'd like it covered in the build.Since the property is the global
spring.http.clients.cookie-handling, it also applies to other Boot-configured HTTP clients in a gateway app, like the redirects default does. I added a short "HTTP Client Defaults" section to the Server MVC docs that mentions both.