Jetty doesn't set the date header on error responses

[markslater]

Jetty version(s)
11.0.15

Java version/vendor (use: java -version)
openjdk version "11.0.18" 2023-01-17
OpenJDK Runtime Environment (build 11.0.18+10-post-Ubuntu-0ubuntu120.04.1)
OpenJDK 64-Bit Server VM (build 11.0.18+10-post-Ubuntu-0ubuntu120.04.1, mixed mode, sharing)

OS type/version

Linux 5.4.0-147-generic

Description

Jetty's default error responses don't include the date header, but RFC 9110 specifies that it's mandatory for an "origin server with a clock".

How to reproduce?

Run the following class:

package com.example;

import org.eclipse.jetty.server.Server;

public class ErrorServer {
    public static void main(String[] args) throws Exception {
        new Server(8080).start();
    }
}

Navigate to http://localhost:8080 in a browser, and view the response headers. I get:

Cache-Control: must-revalidate,no-cache,no-store
Content-Length: 435
Content-Type: text/html;charset=iso-8859-1
Server: Jetty(11.0.15)

This appears to be caused by a call to Response.resetContent() in the default error handling flow, which erases the date header that had previously been set by HttpChannel.onRequest(MetaData.Request).

[gregw]

@markslater your analysis of the cause is indeed correct. We've been aware for a while that resetContent does reset headers that the server itself has generated.... but we'd not acted because it was not clear if that really was the intended semantic of reset or not. However, as you say, RFC9110 makes it pretty clear that we should be including at least the Date header.

Now we have a choice between:

1. preserving it as it was (but an app may have modified it)
2. re-adding server headers in reset.
3. changing to only generate the server headers during commit.

I think 2. is least change and probably the correct semantic for when to set the date header.

@sbordet thoughts?

[gregw]

Ah, we had already changed resetContent to only reset specific headers:

                case CONTENT_LENGTH:
                case CONTENT_ENCODING:
                case CONTENT_LANGUAGE:
                case CONTENT_RANGE:
                case CONTENT_MD5:
                case CONTENT_LOCATION:
                case TRANSFER_ENCODING:
                case CACHE_CONTROL:
                case LAST_MODIFIED:
                case EXPIRES:
                case ETAG:
                case DATE:
                case VARY:

So why is DATE considered a content header? We should definitely take it out of the list.

I think that would be enough. I'm not sure we need to worry about applications that deliberately remove the Date header.

[gregw]

This has been fixed in jetty 10,11 and the ee environments of 12.

However, in the core APIs, there is only a response reset method that removes all headers. We probably need to modify that to restore (or not remove) any server provided headers.

[joakime]

Closing as it is merged