Skip to content

Disable cookie handling by default for the Gateway Server MVC HTTP client - #4352

Open
Asyncronize wants to merge 1 commit into
spring-cloud:mainfrom
Asyncronize:gh-3311-disable-cookie-handling
Open

Asyncronize wants to merge 1 commit into
spring-cloud:mainfrom
Asyncronize:gh-3311-disable-cookie-handling

Conversation

@Asyncronize

Copy link
Copy Markdown

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-Cookie from 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-handling to disable in GatewayHttpClientEnvironmentPostProcessor when the user hasn't set it, the same way spring.http.clients.redirects already defaults to dont-follow. Both defaults now go into the one gatewayHttpClientProperties property source, since adding a second source with the same name would replace the first.

I checked how Boot applies DISABLE for each builder before relying on it: HttpComponents uses StandardCookieSpec.IGNORE, Jetty an empty cookie store, JDK no CookieHandler, and Reactor Netty and the simple factory only reject ENABLE. So disable doesn't break any of them, which was the open question in my comment on the issue.

Tests:

  • GatewayServerMvcAutoConfigurationTests: the default ends up in HttpClientSettings (via SpringApplication, since the post processor doesn't run with ApplicationContextRunner), a user value is kept, and only missing defaults are added.
  • CookieHandlingTests in integration-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, for http-components, reactor, jdk and simple. On main the http-components case fails with session=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-client added 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.

…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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants