org.springframework.http.HttpRequest backwards compatibility

[andrewtaylor]

We recently tried to upgrade our application to Spring Boot 3.4, but encountered an incompatibility with a third party library and the org.springframework.http.HttpRequest interface (from spring-web). It seems in 6.2 the getAttributes method was added, which has broken this third party library we use: https://github.com/mkopylec/charon-spring-boot-starter.

While there is an existing task with that third party library to fix the issue: mkopylec/charon-spring-boot-starter#150, might the solution to be to make getAttributes() a default method to maintain backwards compatibility?

Just a general observation, but was the HttpRequest interface not trying to encapsulate an HTTP request and not a Servlet API request? The current class comment seems to suggest it represents an "HTTP request message" and unless I'm mistaken, attributes are Servlet specific.

[bclozel]

This was added in #32027 for HTTP clients, so not related to the Servlet spec. Also, we have a similar arrangement for Reactive HTTP exchanges.

Do you have a default implementation in mind that would fit the contract? Our Javadoc states that this method returns "a mutable map of request attributes for this request.", so returning Collections.emptyList() won't work here.

[andrewtaylor]

If this was for HTTP clients, could the getAttributes not have gone on ClientHttpRequest? What use case are the attributes solving for say a proxy of actual HTTP requests, which the contract of this interface used to support?

[bclozel]

If this was for HTTP clients, could the getAttributes not have gone on ClientHttpRequest?

I guess it's too late to consider this case since we already released public API. Changing that will break binary compatibility.
I can't think of a default implementation that would not somehow break at runtime one way or another.

I think getting mkopylec/charon-spring-boot-starter#150 merged is now the best course of action here. If this library is a critical part of your stack, and in order to avoid such issues in the future, you could consider testing the Spring Framework milestones as they are announced as this could be caught much earlier in the process.

Sorry for the inconvenience.

[andrewtaylor]

Thank you for taking the time to review this. I had assumed a default implementation could have returned empty map assuming it was documented clearly, or possibly thrown an java.lang.UnsupportedOperationException in order to try and maintain backwards compatibility with 6.x. Neither great solutions as you point out.

[bclozel]

Thanks for this discussion @andrewtaylor . Indeed, returning an empty mutable map will be even more deceiving as you'll happily put attributes there and they'll never be reflected. Same for null as we're working on nullability refinements in our API.