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

GzipHandler fails to set Vary header on 304 responses #8905

Closed
markslater opened this issue Nov 16, 2022 · 1 comment · Fixed by #8906
Closed

GzipHandler fails to set Vary header on 304 responses #8905

markslater opened this issue Nov 16, 2022 · 1 comment · Fixed by #8906
Assignees
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)

Comments

@markslater
Copy link
Contributor

Jetty version(s)
11.0.12

Java version/vendor (use: java -version)
openjdk version "11.0.17" 2022-10-18
OpenJDK Runtime Environment (build 11.0.17+8-post-Ubuntu-1ubuntu220.04)
OpenJDK 64-Bit Server VM (build 11.0.17+8-post-Ubuntu-1ubuntu220.04, mixed mode, sharing)

OS type/version
Linux 5.4.0-120-generic

Description
RFC 9110 states that a 304 response must generate a Vary header matching that generated by the equivalent 200 response:

The server generating a 304 response MUST generate any of the following header fields that would have been sent in a 200 (OK) response to the same request:

Content-Location, Date, ETag, and Vary
Cache-Control and Expires (see [CACHING])

Jetty's GzipHandler adds a Vary header for 200 responses it gzips, but doesn't add it for corresponding 304 responses.

GzipHttpOutputInterceptor, which GzipHandler uses to modify responses, infers from the ETag whether a 304 response it is handling refers to a resource it previously compressed, so perhaps the Vary header could be added at this point?

How to reproduce?
Using the following class:

package com.example;

import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;

import java.io.IOException;
import java.io.Writer;
import java.util.UUID;

public class GzipETagServer {
    public static void main(String[] args) throws Exception {
        final Server server = new Server(8080);
        final ServletContextHandler servletContextHandler = new ServletContextHandler();
        servletContextHandler.addServlet(new ServletHolder(new FooServlet()), "/foo");
        final GzipHandler gzipHandler = new GzipHandler();
        gzipHandler.setHandler(servletContextHandler);
        server.setHandler(gzipHandler);
        server.start();
    }

    private static final class FooServlet extends HttpServlet {

        private final String eTag = '"' + UUID.randomUUID().toString() + '"';

        @Override
        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
            resp.setHeader("etag", eTag);
            if (eTag.equals(req.getHeader("if-none-match"))) {
                resp.setStatus(304);
            } else {
                resp.setStatus(200);
                try (Writer responseWriter = resp.getWriter()) {
                    responseWriter.write("Foo".repeat(100));
                }
            }
        }
    }
}

Visiting http://localhost:8080/foo in a browser will result in a gzipped 200 response with Vary header Vary: Accept-Encoding (and ETag e.g. ETag: "9af0190a-7fb3-4511-873c-75a6fad51345--gzip"). Visiting the same address again will result in a 304 response, but with no Vary header (with the same ETag

@markslater markslater added the Bug For general bugs on Jetty side label Nov 16, 2022
@joakime joakime self-assigned this Nov 16, 2022
@joakime joakime added the Specification For all industry Specifications (IETF / Servlet / etc) label Nov 16, 2022
@joakime joakime moved this from To do to 🔖 Review in progress in Jetty 10.0.13 / 11.0.13 - FROZEN Nov 16, 2022
joakime added a commit that referenced this issue Nov 16, 2022
…dified) responses (per RFC9110)

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor

joakime commented Nov 16, 2022

Opened PR #8906

Repository owner moved this from 🔖 Review in progress to ✅ Done in Jetty 10.0.13 / 11.0.13 - FROZEN Nov 21, 2022
joakime added a commit that referenced this issue Nov 21, 2022
…-304-vary

Issue #8905 - GzipHandler should include `Vary` header on 304 (Not Modified) responses (per RFC9110)
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
None yet
2 participants