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

Return requested content type error response in case of URI Violation and not always text/html #11524

Closed
dhs3000 opened this issue Mar 16, 2024 · 7 comments · Fixed by #11522
Closed
Labels
Bug For general bugs on Jetty side

Comments

@dhs3000
Copy link
Contributor

dhs3000 commented Mar 16, 2024

Jetty version(s)
Jetty 12.0.7

Jetty Environment
core

Java version/vendor (use: java -version)
openjdk version "21.0.2"

Description

When a client requests a path with an URI violation, the server currently always returns a text/html error response regardless of what the client specified in the Accept header, even though the ErrorHandler class checks for that header.

When debugging,. it seems that at that point in the ErrorHandler the request headers are completely empty, maybe unparsed?

How to reproduce?

The following test requests a JSON response content type but fails with

[ERROR]   ServerTest.testGETWithURIViolation:145 
Expected: is "application/json;charset=utf-8"
     but: was "text/html;charset=ISO-8859-1"
public class ServerTest
{
   // A test case added to `jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java`
   
    public void testGETWithURIViolation() throws Exception
    {
        _context.setHandler(new Handler.Abstract()
        {
            @Override
            public boolean handle(Request request, Response response, Callback callback)
            {
                response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain");
                Content.Sink.write(response, true, "Hello", callback);
                return true;
            }
        });
        _server.start();

        String request = """
                GET /path/with//uri/violation HTTP/1.0
                Host: hostname
                Accept: application/json
                
                """;
        HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));

        assertThat(response.getStatus(), is(HttpStatus.BAD_REQUEST_400));
        assertThat(response.getField(HttpHeader.CONTENT_TYPE).getValue(), is("application/json"));
    }

    // ...
}

Expected returned content

{
"cause0":"org.eclipse.jetty.http.BadMessageException: 400: Ambiguous URI empty segment",
"message":"Ambiguous URI empty segment",
"url":"/path/with//uri/violation",
"status":"400"
}

But was following

<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
<title>Error 400 Ambiguous URI empty segment</title>
</head>
<body>
<h2>HTTP ERROR 400 Ambiguous URI empty segment</h2>
<table>
<tr><th>URI:</th><td>/badURI</td></tr>
<tr><th>STATUS:</th><td>400</td></tr>
<tr><th>MESSAGE:</th><td>Ambiguous URI empty segment</td></tr>
<tr><th>CAUSED BY:</th><td>org.eclipse.jetty.http.BadMessageException: 400: Ambiguous URI empty segment</td></tr>
</table>
<h3>Caused by:</h3><pre>org.eclipse.jetty.http.BadMessageException: 400: Ambiguous URI empty segment
        at [email protected]/org.eclipse.jetty.server.internal.HttpConnection$HttpStreamOverHTTP1.headerComplete(HttpConnection.java:1187)
        at [email protected]/org.eclipse.jetty.server.internal.HttpConnection$RequestHandler.headerComplete(HttpConnection.java:953)
        at [email protected]/org.eclipse.jetty.http.HttpParser.parseFields(HttpParser.java:1344)
        at [email protected]/org.eclipse.jetty.http.HttpParser.parseNext(HttpParser.java:1637)
        at [email protected]/org.eclipse.jetty.server.internal.HttpConnection.parseRequestBuffer(HttpConnection.java:568)
        at [email protected]/org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:391)
        at [email protected]/org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
        at [email protected]/org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:99)
        at [email protected]/org.eclipse.jetty.io.ByteArrayEndPoint.lambda$new$0(ByteArrayEndPoint.java:58)
        at [email protected]/org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:971)
        at [email protected]/org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1201)
        at [email protected]/org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1156)
        at java.base/java.lang.Thread.run(Thread.java:1583)
</pre>
<hr/><a href="https://eclipse.org/jetty">Powered by Jetty:// 12.0.8-SNAPSHOT</a><hr/>

</body>
</html>
@dhs3000 dhs3000 added the Bug For general bugs on Jetty side label Mar 16, 2024
@sbordet
Copy link
Contributor

sbordet commented Mar 18, 2024

The error happens very early in the parsing, Jetty finds the URI violation and does not both parsing further, as this can be an attack.

We try to be gentle and send a proper response rather than just closing the connection, but the Accept header has not been parsed so we cannot really return the response according to the original headers.

@dhs3000
Copy link
Contributor Author

dhs3000 commented Mar 18, 2024

@sbordet That is what I thought 👍

@joakime
Copy link
Contributor

joakime commented Mar 18, 2024

@dhs3000 It looks like the error you are seeing is from the "Bad Message" handling (aka 400 Bad Request) in the ErrorHandler.

https://github.com/jetty/jetty.project/blob/jetty-12.0.7/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java#L417-L425

public ByteBuffer badMessageError(int status, String reason, HttpFields.Mutable fields)
{
if (reason == null)
reason = HttpStatus.getMessage(status);
if (HttpStatus.hasNoBody(status))
return BufferUtil.EMPTY_BUFFER;
fields.put(HttpHeader.CONTENT_TYPE, Type.TEXT_HTML_8859_1.asString());
return BufferUtil.toBuffer("<h1>Bad Message " + status + "</h1><pre>reason: " + reason + "</pre>");
}

@dhs3000
Copy link
Contributor Author

dhs3000 commented Mar 18, 2024

In the debugger in my local test project, it arrives in ErrorHandler.generateResponse() but the request headers are completely empty, it then goes into ErrorHandler.generateAcceptableResponse() and ErrorHandler.writeErrorHtml(). The debugger doesn't go into ErrorHandler.badMessageError().

@joakime
Copy link
Contributor

joakime commented Mar 18, 2024

If that code in ErrorHandler was run, then the error occurred after parsing and before handling.

I would expect that the Accept (request) header is present.

Put a breakpoint here and see if this is where your UriCompliance violation triggers -> https://github.com/jetty/jetty.project/blob/jetty-12.0.7/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java#L602

If that is where your BadMessage triggers, then you should have that Accept header.

BTW, Don't test with HTTP/1.0.
Use HTTP/1.1 with Connection: close header instead. (this change should have no impact on your reported behavior)

@dhs3000
Copy link
Contributor Author

dhs3000 commented Mar 19, 2024

It might have been unclear and I updated my initial issue/post trying to clarify: I added (locally) a test case to jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java. The test case is a copy of the existing testSimpleGET() (And therefore uses the same HTTP etc. I tried using HTTP/1.1 and Connection: close header, but it makes no difference.) That test case fails and returns HTML instead of JSON. I added those to the message as well.

One other noteworthy difference is that the generated HTML contains a request URI /badURI instead of /path/with//uri/violation.

@dhs3000
Copy link
Contributor Author

dhs3000 commented Mar 19, 2024

I cannot debug the test case that I added here, as Jetty project is not fully importing in IntelliJ for me, I can only execute it with maven command line.

The test case that I am debugging is in my local project using Kotlin / http4k-jetty. This is not stopping at HttpChannelState#L602 (or not even in org.eclipse.jetty.server.internal.HttpChannelState.HandlerInvoker#run()), but it stops at HttpConnection.java#L1031-L1033.

@sbordet sbordet moved this to 🏗 In progress in Jetty 12.0.8 - FROZEN Mar 19, 2024
@sbordet sbordet linked a pull request Mar 19, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.8 - FROZEN Mar 19, 2024
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
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants