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

HTTP Fields with OWS (Optional WhiteSpace) in value are not properly parsed in Jetty 12 #11659

Closed
alanvanness opened this issue Apr 16, 2024 · 2 comments · Fixed by #11661
Closed
Assignees
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)

Comments

@alanvanness
Copy link

Jetty version(s)
12.0.7

Jetty Environment
ee10

Java version/vendor (use: java -version)
C:\tools\jdk21\bin>java -version
openjdk version "21.0.2" 2024-01-16 LTS
OpenJDK Runtime Environment Corretto-21.0.2.13.1 (build 21.0.2+13-LTS)
OpenJDK 64-Bit Server VM Corretto-21.0.2.13.1 (build 21.0.2+13-LTS, mixed mode, sharing)

OS type/version
Windows 10 Enterprise Version 22H2

Description
Jetty 12's HttpParser throws an exception when parsing an HTTP response that contains multiple spaces before the Content-Length header value. This does not happen with Jetty 11, and the extra space is not in violation of the spec. The exception is:

Caused by: o.e.j.h.BadMessageException: 400: Invalid Content-Length Value
	at o.e.j.h.HttpParser.convertContentLength(HttpParser.java:1138)
	at o.e.j.h.HttpParser.parsedHeader(HttpParser.java:991)
	at o.e.j.h.HttpParser.parseFields(HttpParser.java:1199)
	at o.e.j.h.HttpParser.parseNext(HttpParser.java:1545)
	... 21 common frames omitted
Caused by: j.l.NumberFormatException: null
	... 25 common frames omitted

How to reproduce?
The following JUnit tests reproduce/illustrate the problem:

package jetty;

import org.eclipse.jetty.http.HttpTester;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

/**
* These two tests illustrate an issue in Jetty 12 with respect to parsing a
* response that has more than one space before the Content-Length header value
* (and after the colon).
*/
class JettyHttpParserTest {

       // This test passes in both Jetty 11 and Jetty 12
       // Note that there is only one space before the Content-Length header value in this test 
       @Test
       @DisplayName("Content-Length with no leading spaces")
       void ParseResponseWithNoLeadingSpaces() throws Exception {
              String response = """
              HTTP/1.1 200 OK
              Content-Length: 12
              Content-Type: text/plain
              Hello World!
              """;

              Assertions.assertDoesNotThrow(() -> HttpTester.parseResponse(response));
       }

       // This test passes in Jetty 11, but fails in Jetty 12
       // Note that there are two spaces in front of the Content-Length header value in this test
       @Test
       @DisplayName("Content-Length with leading spaces")
       void ParseResponseWithLeadingSpaces() throws Exception {
              String response = """
              HTTP/1.1 200 OK
              Content-Length:  12
              Content-Type: text/plain
              Hello World!
              """;

              Assertions.assertDoesNotThrow(() -> HttpTester.parseResponse(response));
       }
}

@alanvanness alanvanness added the Bug For general bugs on Jetty side label Apr 16, 2024
@joakime joakime added the Specification For all industry Specifications (IETF / Servlet / etc) label Apr 17, 2024
@joakime
Copy link
Contributor

joakime commented Apr 17, 2024

Looks like we are not ignore OWS before the Content-Length value properly.
PR will be submitted shortly.

@joakime joakime self-assigned this Apr 17, 2024
@joakime joakime moved this to 👀 In review in Jetty 12.0.9 - FROZEN Apr 17, 2024
@joakime joakime moved this from 👀 In review to 🏗 In progress in Jetty 12.0.9 - FROZEN Apr 17, 2024
@joakime joakime changed the title Jetty 12 HttpParser fails to parse a response with more than one space before the Content-Length header value Content-Length with OWS (Optional Whitespace) is not properly parsed in Jetty 12 Apr 17, 2024
@joakime joakime changed the title Content-Length with OWS (Optional Whitespace) is not properly parsed in Jetty 12 Content-Length with OWS (Optional WhiteSpace) is not properly parsed in Jetty 12 Apr 17, 2024
@joakime
Copy link
Contributor

joakime commented Apr 17, 2024

PR #11661 opened

joakime added a commit that referenced this issue Apr 19, 2024
* implemented changes to CACHE and parseFields to handle OWS properly for all UNMATCHED_VALUE headers.
* added 3 new OWS test cases (that fail in 12.0.x HEAD btw) to handle this OWS case.
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.9 - FROZEN Apr 19, 2024
@joakime joakime changed the title Content-Length with OWS (Optional WhiteSpace) is not properly parsed in Jetty 12 HTTP Fields with OWS (Optional WhiteSpace) in value are not properly parsed in Jetty 12 Apr 19, 2024
gregw added a commit that referenced this issue May 18, 2024
Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659
gregw added a commit that referenced this issue May 18, 2024
Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659
gregw added a commit that referenced this issue May 19, 2024
Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659
gregw added a commit that referenced this issue May 21, 2024
Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659
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 Specification For all industry Specifications (IETF / Servlet / etc)
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants