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

ServerWebExchange.mutate() does not delegate modified headers back to calling instance #34067

Closed
Aafi01 opened this issue Dec 11, 2024 · 3 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid

Comments

@Aafi01
Copy link

Aafi01 commented Dec 11, 2024

Prior to spring-web-6.2.0 calling the mutate() method of ServerWebExchange and modifying the request headers would mutate the values in the calling instance of exchange object. This behavior matches the documentation of mutate() method. But this no longer works with version 6.2.0.

Dependency package: spring-web
Last working version: 6.1.13
Issue found in version: 6.2.0

MRE

Java version: 17
Dependencies: spring-boot-starter-webflux:3.4.0 (spring-web:6.2.0), spring-boot-starter-test:3.4.0

Following test fails:

@Test
void mutateShouldDelegateBackModifiedHeadersToThisInstance() {
    var mockServerHttpRequest = MockServerHttpRequest.get("/resource").build();
    var exchange = MockServerWebExchange.from(mockServerHttpRequest);

    String transactionId = UUID.randomUUID().toString();
    exchange.mutate().request(builder -> builder
            .header("Transaction-Id", transactionId));

    String transactionIdFromExchange = exchange.getRequest().getHeaders().getFirst("Transaction-Id");
    Assertions.assertEquals(transactionId, transactionIdFromExchange);
}

Using mutate().build() and using the returned exchange object still works as it did in version 6.1.13.

@Test
void mutateThenBuildShouldReturnModifiedHeaders() {
    var mockServerHttpRequest = MockServerHttpRequest.get("/resource").build();
    var exchange = MockServerWebExchange.from(mockServerHttpRequest);

    String transactionId = UUID.randomUUID().toString();
    var mutatedExchange = exchange.mutate().request(builder -> builder
            .header("Transaction-Id", transactionId)
    ).build();

    String transactionIdFromExchange = mutatedExchange.getRequest().getHeaders().getFirst("Transaction-Id");
    Assertions.assertEquals(transactionId, transactionIdFromExchange);
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 11, 2024
@bclozel bclozel added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Dec 11, 2024
@bclozel
Copy link
Member

bclozel commented Dec 11, 2024

I think this was a bug and this was fixed in #32097 by chance.

The second behavior is the expected one and mutating a new instance of the exchange should not mutate the original headers. It's a bit late in the 6.1.x cycle to consider a backport so we'll leave it at that for now.

Thanks for the detailed report and the repro. Unfortunately, we can't reinstate this behavior as this was incorrect.
This behavior is now tested with #33666 so there's no need for additional tests. I'll close this issue now.

Thanks!

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2024
@bclozel bclozel added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 11, 2024
@Aafi01
Copy link
Author

Aafi01 commented Dec 11, 2024

Thanks for the triage. Quite a pesky bug to remain undetected from spring 5.x.x to all the way till 6.2.0. I can live with the second behavior, but it might be good idea to update the javadoc for this method. It specifically mentions "returning either mutated values or delegating back to this instance". Or I misunderstood what that meant.

https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/server/ServerWebExchange.html#mutate()

@bclozel
Copy link
Member

bclozel commented Dec 11, 2024

I think this means that the decorator will either return a new value configured with the mutated version or return the original value. I guess it's an implementation details and we can improve that bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants