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

Jetty 12 HTTP SPI does not preserve double-quotes on valid request headers #10500

Closed
AlexanderSchuetz97 opened this issue Sep 12, 2023 · 7 comments · Fixed by #10503
Closed
Labels
Bug For general bugs on Jetty side

Comments

@AlexanderSchuetz97
Copy link

AlexanderSchuetz97 commented Sep 12, 2023

Jetty version(s)
12.01

Jetty Environment
ee10 and org.eclipse.jetty.http.spi

Java version/vendor (use: java -version)
Java 17, reproducable with any Java 17 version.

OS type/version
Windows 11

Description
A Soap request sets the following header value:

Content-Type: multipart/related;start="<rootpart*[email protected]>";type="application/xop+xml";boundary="uuid:dcb36243-479b-418e-868e-9681dcbedd73";start-info="application/soap+xml;action=\"urn:ihe:iti:2007:ProvideAndRegisterDocumentSet-b\""

It is of paramount importance that this header value is preserved AS-IS otherwise the soap request will fail.
Jetty will remove all " chars in this header value.

org.eclipse.jetty.http.spi.JettyHttpExchangeDelegate will do this in the getRequestHeaders() method.

Debugging yielded the following:
field.getValue() does return the correct header value with all quotes.
field.getValues() does returns a Collection with 1 element.

The one element is my header but without all my very important " marks.
Upon further inspection it appears that
QuotedCSV list = new QuotedCSV(false, value);
in org.eclipse.jetty.http.HttpField#getValueList removes my " marks.

This behavior did not occur in jetty 11.0.15 and breaks all my soap servers.

My Soap Endpoints are registered via normal Endpoint.publish means and that works with Both Jetty 11 and Jetty 12.
The Issue of removal of " only occurs in Jetty 12.

The entire getRequestHeaders() method is called by
com.sun.xml.ws.transport.http.server.ServerConnectionImpl#getRequestHeader
of com.sun.xml.ws:jaxws-rt.2.3.2
The code that calls JettyHttpExchangeDelegate looks like this:

return httpExchange.getRequestHeaders().getFirst(headerName);

headerName is as mentioned before "Content-Type"

How to reproduce?

Do anything involving org.eclipse.jetty.http.spi.JettyHttpExchangeDelegate and set the Content-Type header to
multipart/related;start="<rootpart*[email protected]>";type="application/xop+xml";boundary="uuid:dcb36243-479b-418e-868e-9681dcbedd73";start-info="application/soap+xml;action=\"urn:ihe:iti:2007:ProvideAndRegisterDocumentSet-b\""

@AlexanderSchuetz97 AlexanderSchuetz97 added the Bug For general bugs on Jetty side label Sep 12, 2023
@joakime
Copy link
Contributor

joakime commented Sep 12, 2023

Incidentally, your Content-Type value violates the HTTP/1.1 spec.

The \" seen in action=\"urn: consists of 2 characters that are not allowed in a quoted-string segment of a Media-Type definition.

  • \ (char %x5C) is not allowed as a token or in a quoted-string
  • " (char %x22) is not allowed in a quoted-string

See:

Jetty fails to parse this header fully, and this failure is one of the root causes for this issue.

@joakime
Copy link
Contributor

joakime commented Sep 12, 2023

Opened PR #10503 to address this

@joakime joakime changed the title Jetty 12 HTTP SPI mangles header values of SOAP requests Jetty 12 HTTP SPI does not preserve quotes on valid request headers Sep 12, 2023
@joakime joakime changed the title Jetty 12 HTTP SPI does not preserve quotes on valid request headers Jetty 12 HTTP SPI does not preserve double-quotes on valid request headers Sep 12, 2023
@AlexanderSchuetz97
Copy link
Author

AlexanderSchuetz97 commented Sep 12, 2023

I have checked your PR. new QuotedCSV(true, field.getValue()).getValues() in a debugger evaluation would indeed contain the header value with all " intact.

I do not know why this "violates" the http1/1 spec.
A soap client made this request. As you are surely aware, soap is kind of a take it or leave it situation.
I appreciate you resolving this quickly and will look forward to jetty 12.0.2.
For now i will downgradeto jetty 11.0.6.

Also I am not entirely sure I 100% copied the header value from the debugger properly (in regards to the backslash and " situation) I am pretty sure i forogt a trailing " in my original comment so i have appended it. I do not think it matters that much. I did not check with wireshark what was actually sent via the TCP socket. I would prefer to not have to do that...

@joakime
Copy link
Contributor

joakime commented Sep 12, 2023

I have checked your PR. new QuotedCSV(true, field.getValue()).getValues() in a debugger evaluation would indeed contain the header value with all " intact.

The prior HttpFields.getValues() call removes the double-quotes, as designed.
Using QuotedCSV with a quote preserving option was the way around this.

I do not know why this "violates" the http1/1 spec. A soap client made this request. As you are surely aware, soap is kind of a take it or leave it situation. I appreciate you resolving this quickly and will look forward to jetty 12.0.2. For now i will downgradeto jetty 11.0.6.

Surely you meant 11.0.16, not 11.0.6. (you don't want to downgrade to something vulnerable).

The errors from QuotedCSV were pointing to the invalid \ and " in the state token (not double-quoted enclosed)

Also I am not entirely sure I 100% copied the header value from the debugger properly (in regards to the " situation) I am pretty sure i forogt a trailing " in my original comment so i have appended it. I do not think it matters that much. I did not check with wireshark what was actually sent via the TCP socket. I would prefer to not have to do that...

This missing end quote causes the media-type parameter to be seen as a token, not a quoted-string per spec. (to use quoted-string you MUST have a double-quote at the start and end of the parameter)

This is outlined in the following spec sections.

Without that end quote, the parameter parsing flags the \" as invalid.

If you notice the test cases in the PR, the proper quoting is used to satisfy the quoted-string parameter requirements of the HTTP/1.1 spec.

https://github.com/eclipse/jetty.project/blob/fe56807d00bbfce50d6eca2743ce3b61ffc89c35/jetty-core/jetty-http-spi/src/test/java/org/eclipse/jetty/http/spi/SPIServerTest.java#L131-L135

In short, leave off that end double quote and double quote parsing rules for quoted-string will fail that header value and the results will be completely unpredictable.

@joakime
Copy link
Contributor

joakime commented Sep 12, 2023

@AlexanderSchuetz97
Copy link
Author

AlexanderSchuetz97 commented Sep 12, 2023

Yeah you are correct. I meant 11.0.16.
I was originally at 11.0.15 looking to update. I decided to give jetty 12 a try.
But cant due to this issue.
I will await the next jetty 12 minor version and then try to update to 12 again.

If there is any further regression in jetty that prevents com.sun.xml.ws:jaxws-rt.2.3.2 SOAP Endpoints from working properly
then I will make a new issue after the next jetty 12 version.

Thanks for your help

joakime added a commit that referenced this issue Sep 13, 2023
joakime added a commit that referenced this issue Sep 13, 2023
joakime added a commit that referenced this issue Sep 13, 2023
joakime added a commit that referenced this issue Sep 14, 2023
joakime added a commit that referenced this issue Sep 14, 2023
joakime added a commit that referenced this issue Sep 15, 2023
…`JettyHttpExchangeDelegate` (#10503)

* Issue #10500 - preserve request header quoting when accessed through JettyHttpExchangeDelegate
* improve test cases with quoted-pair feature in RFC
* add skip of value lists on specific headers known to not have value lists
* Remove URL usage
* Changes from review
@AlexanderSchuetz97
Copy link
Author

works with 12.0.2 thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
2 participants