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

Spring Framework 5.3.x is incompatible with Jetty 10 (Client) #29867

Closed
mikebell90 opened this issue Jan 20, 2023 · 4 comments
Closed

Spring Framework 5.3.x is incompatible with Jetty 10 (Client) #29867

mikebell90 opened this issue Jan 20, 2023 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@mikebell90
Copy link
Contributor

We have recently moved to Spring Boot 2.7.5 with Jetty 10 (or are trying to).

What we found was two things - one is probably not relevant, that you have to manually manage the reactive-httpclient adapter for jetty to 2.x (would be nice if this were documented), but more importantly, the JettyHeadersAdapter class in org.spring.framework.http.client.reactive is compatible with Jetty 9 but not Jetty 10.

We received errors like :

java.lang.IncompatibleClassChangeError: Found interface org.eclipse.jetty.http.HttpFields, but class was expected
	at org.springframework.http.client.reactive.JettyHeadersAdapter.toSingleValueMap(JettyHeadersAdapter.java:86)

digging further, we find the JettyHeaderAdapter class imports org.eclipse.jetty.http.HttpFields and indeed this changed from a class in Jetty 9 to an interface in Jetty 10+.

methods such as

public void add(String key, @Nullable String value) {
		this.headers.add(key, value); // headers is an HttpFields instance
	}

fail because of this

You fixed issues for Jetty 11/12 which were identical in https://github.com/spring-projects/spring-framework/blob/d84ca2ba90d27a7c63d7b35a6259b5b9cf341118/spring-web/src/main/java/org/springframework/http/client/reactive/JettyHeadersAdapter.java

however this fix (which would be more involved because it would have to sniff for Jetty 10 and instantiate a different JettyHeadersAdapter) has not been backported.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 20, 2023
@jhoeller
Copy link
Contributor

We actually have such Jetty 10 sniffing and a corresponding JettyHeadersAdapter bypass for the server variant in 5.3.x, just not for the client part, it seems. Let's see what we can do about it for Jetty 10 on the client side.

@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 23, 2023
@jhoeller jhoeller added this to the 5.3.26 milestone Jan 23, 2023
@poutsma poutsma self-assigned this Jan 26, 2023
@poutsma
Copy link
Contributor

poutsma commented Jan 26, 2023

We have recently moved to Spring Boot 2.7.5 with Jetty 10 (or are trying to).

@mikebell90 We always recommend upgrading to the latest version of Spring Boot, which is currently 2.7.8. Earlier versions might lack important security fixes.

We actually have such Jetty 10 sniffing and a corresponding JettyHeadersAdapter bypass for the server variant in 5.3.x, just not for the client part, it seems.

@jhoeller We do, see https://github.com/spring-projects/spring-framework/blob/5.3.x/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java#L80, and in my personal tests with Boot 2.7.8 and Jetty 10.0.13, it seems to work fine.

@mikebell90 I cannot reproduce this issue. You might want to check your classpath, and make sure that it only has Jetty 10 jars on it. Otherwise, can you provide a complete, minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces this problem?

@poutsma poutsma added the status: waiting-for-feedback We need additional information before we can continue label Jan 26, 2023
@mikebell90
Copy link
Contributor Author

But what about

return HttpHeaders.readOnlyHttpHeaders(new JettyHeadersAdapter(this.jettyRequest.getHeaders()));

Which does not have such a sniff

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 26, 2023
poutsma added a commit that referenced this issue Jan 26, 2023
Though Jetty 10 was previously supported in the JettyClientHttpResponse,
this commit ensures support in the JettyClientHttpRequest.

Closes gh-29867
See gh-26123
@poutsma
Copy link
Contributor

poutsma commented Jan 26, 2023

Which does not have such a sniff

Indeed, though that code is pretty hard to trigger, and is only invoked when inspecting the request post exchange.

At any rate, this is now fixed, see 21c3d4f

@poutsma poutsma closed this as completed Jan 26, 2023
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: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants