Support Servlet 4.0 request trailerFields

[niloc132]

This enables applications and libraries to use trailers according to the
servlet 4.0 spec instead of casting to Jetty's custom implementation.

Roughly backported from Jetty 10.x, but with @OverRide omitted, and some
simple javadoc since none will be inherited.

Signed-off-by: Colin Alworth colin@colinalworth.com

[joakime]

Jetty 9.4.x is in maintenance mode.
It is taking bug fixes and security fixes at the moment, not new functionality.

Active branches for new functionality is Jetty 10.0.x and Jetty 11.0.x.

If you want Servlet 4.0 behavior you have to be using Jetty 10.0.x (which supports Servlet 4.0).
Jetty 9.4.x is for Servlet 3.1.0.

Also note that javax.servlet.* is dead, you should move to jakarta.servlet.* if you are doing new development.

[niloc132]

Understood, my motivation is only that even today, Java 8 is still widely deployed, with many projects stuck in the chicken/egg situation of updating. I appreciate that Java 10/11 have moved up to Java 11 (and to jakarta.servlet in jetty 11), but am still personally bound to support specific requirements for the time being. My hope was that since Jetty 10/11 do not support setTrailers (and indeed a javax.servlet 4.0 project can't use jetty as a direct dependency), this would smooth the migration path for other projects.

[joakime]

@niloc132 know that Jakarta EE 10 (due out before the end of the year) and even Spring 6 (due out next year) are setting JDK 17 as the minimum JVM.

The struggle to upgrade is becoming even more intense.

[joakime]

My hope was that since Jetty 10/11 do not support setTrailers

Both Jetty 10 on Servlet 4.0 and Jetty 11 on Servlet 5.0 support the Servlet spec defined HttpServletResponse.setTrailerFields(Supplier<Map<String, String>> supplier) and HttpServletResponse.getTrailerFields() APIs today.

[niloc132]

Yes - I initially wrote the downstream code for the standard setTrailerFields method, and it only failed in Jetty 9.4 (since left unimplemented, the default method fails silently, and h2 streams, otherwise well-supported in jetty 9.4, incorrectly close without delivering trailers). Hacky attempts at an instanceof Response and cast failed in Jetty 10+, since the internal method was removed. Backwards compatibility is unfortunately trying to find a way to strike a balance between competing priorities.

I appreciate you taking the time to discuss this, and understand your perspective.

[gregw]

Note that this PR doesn't prevent an app from needing to downcast the response instance to a Jetty Response. It is really only allowing an app to have the same code run on jetty 9 and 10.
You can probably achieve the same by creating a HttpResponseWrapper class that does the downcast and makes the getTrailerFields method available, and that would not need any jetty modifications. Does that work for you?

[niloc132]

@gregw I'm not sure I understand how your idea would work - were you thinking of something like this?

    // Jetty 9 hack
    if (resp.getClass().getName().equals("org.eclipse.jetty.server.Response")) {
      resp = new HttpServletResponseWrapper(resp) {
        private Supplier<Map<String, String>> trailerFields;
        @Override
        public void setTrailerFields(Supplier<Map<String, String>> supplier) {
          trailerFields = supplier;
        }

        @Override
        public Supplier<Map<String, String>> getTrailerFields() {
          return trailerFields;
        }
      };
    }
    AsyncContext asyncCtx = req.startAsync(req, resp);

That doesn't work for Jetty 10/11 since it is expected that super.setTrailerFields is called - adding that makes a certain amount of sense (and would be a no-op for Jetty 9). For Jetty 9 though it still wouldn't work, since getTrailerFields is never called, instead the jetty-specific Response.setTrailers must be called so that Response.newResponseMetaData() will function. To achieve that, we would have to reflect to check for an invoke setTrailers.

Probably I misunderstood you, and there is an easier way you're thinking such that an overridden getTrailerFields is handled correctly in Jetty 10/11, and somehow that Jetty 9 would know to check that method at all?

Next in response to your suggestion I tried making a Resonse subclass, so that at least when i call req.startAsync(req, replacementResponse) it would have a real jetty response instance, but this also doesn't work - I can't be certain, but garbage data is coming out for some reason on the other side of the wire (both responses writing to output perhaps?). I specifically wired this up using some reflection to see if a) it was a jetty Response, and b) if Response has a setTrailers method I could try to call in the subclass:

  private static HttpServletResponse wrap(HttpServletResponse response) {
    if (!response.getClass().getName().equals("org.eclipse.jetty.server.Response")) {
      return response;
    } else {
      Response r = (Response) response;
      if (JettyResponse.checkJettyTrailers(r)) {
        return new JettyResponse(r.getHttpChannel(), r.getHttpOutput());
      }
      return response;
    }
  }

  private static class JettyResponse extends Response {
    private static volatile boolean haveLooked = false;
    private static volatile Method responseSetTrailers;

    /**
     * @return true if we need to use the jetty9 setTrailers
     */
    private static boolean checkJettyTrailers(Response response) {
      if (haveLooked) {
        return responseSetTrailers != null;
      }
      synchronized (JettyResponse.class) {
        if (haveLooked) {
          return responseSetTrailers != null;
        }
        try {
          responseSetTrailers = response.getClass().getMethod("setTrailers", Supplier.class);
          if (!Modifier.isPublic(responseSetTrailers.getModifiers())) {
            responseSetTrailers = null;
            return false;
          }
          return true;
        } catch (NoSuchMethodException e) {
          return false;
        } finally {
          haveLooked = true;
        }
      }
    }

    public JettyResponse(HttpChannel channel, HttpOutput out) {
      super(channel, out);
    }

    @Override
    public void setTrailerFields(Supplier<Map<String, String>> trailers) {
      try {
        responseSetTrailers.invoke(this, (Supplier<HttpFields>) () -> {
          HttpFields fields = new HttpFields();
          for (Map.Entry<String, String> entry : trailers.get().entrySet()) {
            fields.add(entry.getKey(), entry.getValue());
          }
          return fields;
        });
      } catch (IllegalAccessException e) {
        //public in jetty9, so not possible
        e.printStackTrace();
      } catch (InvocationTargetException e) {
        //TODO test for committed and throw correctly
        throw new RuntimeException(e);
      }
    }
  }

At this point, unless you were suggesting some other way (probably closer to the first method) to intercept and propagate trailers, I'm not sure I understand. My "fix" at this point probably has to be the same reflection code, but specifically at the normal HttpServletResponse.setTrailerFields callsite, and always compile the library against jetty as a compile-only dependency. Could possibly use just Jetty 9 and either a method handle or test if the method is there with reflection, then rely on bytecode for actual runtime dispatch.

[niloc132]

This does appear to work, but as above, requires reflection and a HttpServletResponseWrapper, and including Jetty on the compile-only classpath (but omitting it in the runtime classpath does indeed work as expected).

  /**
   * Helper to optionally wrap Jetty9 HttpServletResponse instances to enable their
   * setTrailerFields implementation to delegate to setTrailers.
   *
   * <p>This class needs to be compiled with Jetty9+ on the classpath, but the static
   * {@link JettyHttpServletResponse#wrap} method is safe to use without jetty being
   * present at runtime.</p>
   */
  private static class JettyHttpServletResponse extends HttpServletResponseWrapper {
    private static volatile boolean haveLooked = false;
    private static volatile Method responseSetTrailers;

    /**
     * Helper to deal with Jetty 9.4.x not supporting the standard implementation
     * of trailers - this will return an HttpServletResponse instance that can safely
     * be used during the rest of this response.
     *
     * @param response the response object provided by the servlet container
     * @return a response object that will be able to correctly handle Servlet 4.0
     * trailers.
     */
    public static HttpServletResponse wrap(HttpServletResponse response) {
      if (!response.getClass().getName().equals("org.eclipse.jetty.server.Response")) {
        // If this isn't from jetty, assume it works as expected and use it
        return response;
      } else {
        Response r = (Response) response;
        // Since this is from Jetty, check if we must use our own response wrapper
        // to let setTrailerFields work properly
        if (checkJettyTrailers(r)) {
          return new JettyHttpServletResponse(r);
        }

        // If not, assume we're using a more recent Jetty that will work properly
        return response;
      }
    }

    /**
     * @return true if we need to use the jetty9 setTrailers
     */
    private static boolean checkJettyTrailers(Response response) {
      if (haveLooked) {
        // if we already checked, return if we found it or not
        return responseSetTrailers != null;
      }
      synchronized (JettyHttpServletResponse.class) {
        if (haveLooked) {
          return responseSetTrailers != null;
        }
        try {
          responseSetTrailers = response.getClass().getMethod("setTrailers", Supplier.class);
          if (!Modifier.isPublic(responseSetTrailers.getModifiers())) {
            responseSetTrailers = null;
            return false;
          }
          return true;
        } catch (NoSuchMethodException e) {
          return false;
        } finally {
          haveLooked = true;
        }
      }
    }

    private final Response resp;

    private JettyHttpServletResponse(Response resp) {
      super(resp);
      this.resp = resp;
    }

    @Override
    public void setTrailerFields(Supplier<Map<String, String>> supplier) {
      try {
        if (resp.isCommitted()) {
          throw new IllegalStateException("Response already committed");
        }
        if (resp.getHttpChannel().getRequest().getHttpVersion().ordinal() <= HttpVersion.HTTP_1_0.ordinal()) {
          throw new IllegalStateException("Cannot set trailers for http version < 1.0");
        }

        responseSetTrailers.invoke(resp, (Supplier<HttpFields>) () -> {
          Map<String, String> map = supplier.get();
          if (map == null) {
            return null;
          }
          HttpFields fields = new HttpFields();
          for (Map.Entry<String, String> entry : map.entrySet()) {
            fields.add(entry.getKey(), entry.getValue());
          }
          return fields;
        });
      } catch (IllegalAccessException e) {
        // already checked that it is public, so shouldn't be possible
        throw new IllegalStateException("Response.setTrailers was not public as expected", e);
      } catch (InvocationTargetException e) {
        // rethrow any permitted runtime exception
        if (e.getCause() instanceof RuntimeException) {
          throw (RuntimeException) e.getCause();
        }

        // Wrap and throw
        throw new IllegalStateException("Unexpected error when calling Jetty 9 Response.setTrailers", e);
      }
    }

    @Override
    public Supplier<Map<String, String>> getTrailerFields() {
      return () -> resp.getTrailers().get().stream()
              .collect(Collectors.toMap(HttpField::getName, HttpField::getValue));
    }
  }

[joakime]

On a standard WebAppContext, using Jetty 9.4.x, if you use the jakarta.servlet-api.4.0.jar in your WEB-INF/lib then it will not be used, only the one from the container will be used (standard servlet spec behavior that).
This PR still requires casting to the Jetty API Response class to function.

Don't be fooled by the classloaders in your unit test cases, as that is not how a runtime on Jetty 9.4.x functions.

Also ...

if (!response.getClass().getName().equals("org.eclipse.jetty.server.Response")) {

That has no guarantee to work, the response object can easily be wrapped by various components (In Jetty and 3rd party libs) and would not appear to be a org.eclipse.jetty.server.Response object.

You have to identify wrapping and unwrap to the root instance at a minimum.
Test if it implements HttpServletResponseWrapper or ServletResponseWrapper and then use ServletResponseWrapper.getResponse() recursively until you have something that is no longer a wrapper.

[niloc132]

I readily acknowledge that this snippet is incomplete, merely that for trivial cases, with only ~2x the lines of code as the PR itself, that it works.

The PR does not require casting to the Jetty API - the hacky snippet does, but only after it has done its simple "typecheck", and has been tested to confirm that it works on classpaths with no jetty versions of any kind present. I haven't changed the PR (as there was no feedback to its content), but only put up the PoC workaround that I am currently exploring in lieu of the actual PR.

[joakime]

  private static class JettyHttpServletResponse extends HttpServletResponseWrapper {
...
    @Override
    public void setTrailerFields(Supplier<Map<String, String>> supplier) {

This would neither compile nor execute on Jetty 9.4.x, as it's a forced Servlet 3.1.0 environment.
You cant upgrade the Servlet spec like that on Jetty 9.4.x.

[niloc132]

This would neither compile nor execute on Jetty 9.4.x, as it's a forced Servlet 3.1.0 environment.

As a project using this workaround would necessarily assume servlet 4.0, both servlet 4.0 and jetty would be on the compile classpath. None of the code in my comments would make sense to be added to Jetty itself, these are only workarounds if the PR's patch is not acceptable (as the "right" way of doing it, just too late to merge to Jetty 9).

The patch attached to the PR itself solved this by avoiding using @Override.

[joakime]

Here's a demonstration.

The project for the war can be found at https://github.com/joakime/use-servlet-4

You can easily create a bare-bones ${jetty.base} that looks like this.

[servlet-4-demo-base]$ tree
.
├── start.ini
└── webapps
    └── use-servlet-4.war

1 directory, 2 files
[servlet-4-demo-base]$ cat start.ini 
--module=deploy
--module=http

[servlet-4-demo-base]$ jar -tvf webapps/use-servlet-4.war 
    95 Fri Sep 24 16:02:36 CDT 2021 META-INF/MANIFEST.MF
     0 Fri Sep 24 16:02:36 CDT 2021 META-INF/
     0 Fri Sep 24 16:02:36 CDT 2021 WEB-INF/
     0 Fri Sep 24 16:02:36 CDT 2021 WEB-INF/lib/
     0 Fri Sep 24 16:02:36 CDT 2021 WEB-INF/classes/
     0 Fri Sep 24 16:02:36 CDT 2021 WEB-INF/classes/demo/
 82973 Fri Nov 13 13:31:04 CST 2020 WEB-INF/lib/jakarta.servlet-api-4.0.4.jar
   563 Fri Sep 24 16:02:32 CDT 2021 WEB-INF/web.xml
  3371 Fri Sep 24 16:02:36 CDT 2021 WEB-INF/classes/demo/DumpServlet.class
    52 Fri Sep 24 15:55:22 CDT 2021 index.jsp
  2019 Fri Sep 24 15:55:50 CDT 2021 META-INF/maven/demo/use-servlet-4/pom.xml
    90 Fri Sep 24 16:02:36 CDT 2021 META-INF/maven/demo/use-servlet-4/pom.properties

You'll notice that WEB-INF/lib/jakarta.servlet-api-4.0.4.jar is present in the WAR file.

The DumpServlet is the following code ...

package demo;

import java.io.IOException;
import java.io.PrintWriter;
import java.lang.reflect.Method;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.security.AccessController;
import java.security.CodeSource;
import java.security.PrivilegedAction;
import java.security.ProtectionDomain;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

public class DumpServlet extends HttpServlet
{
    @Override
    protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
    {
        PrintWriter out = resp.getWriter();
        Class<?> clazz = resp.getClass();
        out.printf("HttpServletResponse = %s (%s)%n", clazz.getName(), getLocation(clazz));
        out.printf("HttpServletResponse methods%n");
        for (Method method : clazz.getMethods())
        {
            out.printf("  %s%n", method);
        }
    }

    private String getLocation(Class<?> clazz)
    {
        return getCodeSourceLocation(clazz).toASCIIString();
    }

    public static URI getCodeSourceLocation(Class<?> clazz)
    {
        try
        {
            ProtectionDomain domain = AccessController.doPrivileged((PrivilegedAction<ProtectionDomain>)() -> clazz.getProtectionDomain());
            if (domain != null)
            {
                CodeSource source = domain.getCodeSource();
                if (source != null)
                {
                    URL location = source.getLocation();

                    if (location != null)
                    {
                        return location.toURI();
                    }
                }
            }
        }
        catch (URISyntaxException ignored)
        {
        }
        return null;
    }
}

Start the server ...

[servlet-4-demo-base]$ java -jar ~/distros/jetty-home-9.4.43.v20210629/start.jar 
2021-09-24 16:10:41.881:INFO::main: Logging initialized @366ms to org.eclipse.jetty.util.log.StdErrLog
2021-09-24 16:10:42.101:INFO:oejs.Server:main: jetty-9.4.43.v20210629; built: 2021-06-30T11:07:22.254Z; git: 526006ecfa3af7f1a27ef3a288e2bef7ea9dd7e8; jvm 11.0.12+7
2021-09-24 16:10:42.115:INFO:oejdp.ScanningAppProvider:main: Deployment monitor [file:///home/joakim/code/jetty/distros/bases/servlet-4-demo-base/webapps/] at interval 1
2021-09-24 16:10:42.188:INFO:oejw.StandardDescriptorProcessor:main: NO JSP Support for /use-servlet-4, did not find org.eclipse.jetty.jsp.JettyJspServlet
2021-09-24 16:10:42.194:INFO:oejs.session:main: DefaultSessionIdManager workerName=node0
2021-09-24 16:10:42.194:INFO:oejs.session:main: No SessionScavenger set, using defaults
2021-09-24 16:10:42.194:INFO:oejs.session:main: node0 Scavenging every 660000ms
2021-09-24 16:10:42.214:INFO:oejsh.ContextHandler:main: Started o.e.j.w.WebAppContext@359df09a{Servlet 4.0 Demo,/use-servlet-4,file:///tmp/jetty-0_0_0_0-8080-use-servlet-4_war-_use-servlet-4-any-3432559648625761543/webapp/,AVAILABLE}{/home/joakim/code/jetty/distros/bases/servlet-4-demo-base/webapps/use-servlet-4.war}
2021-09-24 16:10:42.230:INFO:oejs.AbstractConnector:main: Started ServerConnector@4dc22657{HTTP/1.1, (http/1.1)}{0.0.0.0:8080}
2021-09-24 16:10:42.232:INFO:oejs.Server:main: Started @717ms

Starts fine.

Now hit URL http://localhost:8080/use-servlet-4/

You'll get the following output ...

HttpServletResponse = org.eclipse.jetty.server.Response (file:/home/joakim/code/jetty/distros/jetty-home-9.4.43.v20210629/lib/jetty-server-9.4.43.v20210629.jar)
HttpServletResponse methods
  public void org.eclipse.jetty.server.Response.errorClose()
  public boolean org.eclipse.jetty.server.Response.isIncluding()
  public void org.eclipse.jetty.server.Response.sendProcessing() throws java.io.IOException
  public void org.eclipse.jetty.server.Response.setStatusWithReason(int,java.lang.String)
  public boolean org.eclipse.jetty.server.Response.isWriting()
  public boolean org.eclipse.jetty.server.Response.isStreaming()
  public boolean org.eclipse.jetty.server.Response.isAllContentWritten(long)
  public void org.eclipse.jetty.server.Response.closeOutput() throws java.io.IOException
  public long org.eclipse.jetty.server.Response.getLongContentLength()
  public void org.eclipse.jetty.server.Response.setLongContentLength(long)
  public void org.eclipse.jetty.server.Response.resetForForward()
  public void org.eclipse.jetty.server.Response.setTrailers(java.util.function.Supplier)
  public long org.eclipse.jetty.server.Response.getContentCount()
  public java.lang.String org.eclipse.jetty.server.Response.getContentType()
  public java.util.Collection org.eclipse.jetty.server.Response.getHeaders(java.lang.String)
  public void org.eclipse.jetty.server.Response.setContentType(java.lang.String)
  public void org.eclipse.jetty.server.Response.setContentLength(int)
  public void org.eclipse.jetty.server.Response.include()
  public java.lang.String org.eclipse.jetty.server.Response.getHeader(java.lang.String)
  public void org.eclipse.jetty.server.Response.setBufferSize(int)
  public void org.eclipse.jetty.server.Response.setLocale(java.util.Locale)
  public long org.eclipse.jetty.server.Response.getContentLength()
  public java.lang.String org.eclipse.jetty.server.Response.getReason()
  public java.lang.String org.eclipse.jetty.server.Response.toString()
  public static javax.servlet.http.HttpServletResponse org.eclipse.jetty.server.Response.unwrap(javax.servlet.ServletResponse)
  public void org.eclipse.jetty.server.Response.reset()
  public void org.eclipse.jetty.server.Response.flushBuffer() throws java.io.IOException
  public javax.servlet.ServletOutputStream org.eclipse.jetty.server.Response.getOutputStream() throws java.io.IOException
  public java.util.Locale org.eclipse.jetty.server.Response.getLocale()
  public void org.eclipse.jetty.server.Response.included()
  public void org.eclipse.jetty.server.Response.sendError(int) throws java.io.IOException
  public void org.eclipse.jetty.server.Response.sendError(int,java.lang.String) throws java.io.IOException
  public boolean org.eclipse.jetty.server.Response.isCommitted()
  public void org.eclipse.jetty.server.Response.setCharacterEncoding(java.lang.String)
  public void org.eclipse.jetty.server.Response.setContentLengthLong(long)
  public int org.eclipse.jetty.server.Response.getBufferSize()
  public void org.eclipse.jetty.server.Response.resetBuffer()
  public void org.eclipse.jetty.server.Response.addCookie(org.eclipse.jetty.http.HttpCookie)
  public void org.eclipse.jetty.server.Response.addCookie(javax.servlet.http.Cookie)
  public boolean org.eclipse.jetty.server.Response.containsHeader(java.lang.String)
  public java.lang.String org.eclipse.jetty.server.Response.encodeURL(java.lang.String)
  public java.lang.String org.eclipse.jetty.server.Response.encodeRedirectURL(java.lang.String)
  public java.lang.String org.eclipse.jetty.server.Response.encodeUrl(java.lang.String)
  public java.lang.String org.eclipse.jetty.server.Response.encodeRedirectUrl(java.lang.String)
  public void org.eclipse.jetty.server.Response.sendRedirect(java.lang.String,boolean) throws java.io.IOException
  public void org.eclipse.jetty.server.Response.sendRedirect(int,java.lang.String,boolean) throws java.io.IOException
  public void org.eclipse.jetty.server.Response.sendRedirect(int,java.lang.String) throws java.io.IOException
  public void org.eclipse.jetty.server.Response.sendRedirect(java.lang.String) throws java.io.IOException
  public void org.eclipse.jetty.server.Response.setDateHeader(java.lang.String,long)
  public void org.eclipse.jetty.server.Response.addDateHeader(java.lang.String,long)
  public void org.eclipse.jetty.server.Response.setHeader(org.eclipse.jetty.http.HttpHeader,java.lang.String)
  public void org.eclipse.jetty.server.Response.setHeader(java.lang.String,java.lang.String)
  public void org.eclipse.jetty.server.Response.addHeader(java.lang.String,java.lang.String)
  public void org.eclipse.jetty.server.Response.setIntHeader(java.lang.String,int)
  public void org.eclipse.jetty.server.Response.addIntHeader(java.lang.String,int)
  public int org.eclipse.jetty.server.Response.getStatus()
  public java.util.Collection org.eclipse.jetty.server.Response.getHeaderNames()
  public java.lang.String org.eclipse.jetty.server.Response.getCharacterEncoding()
  public java.io.PrintWriter org.eclipse.jetty.server.Response.getWriter() throws java.io.IOException
  public void org.eclipse.jetty.server.Response.setStatus(int,java.lang.String)
  public void org.eclipse.jetty.server.Response.setStatus(int)
  public org.eclipse.jetty.http.HttpFields org.eclipse.jetty.server.Response.getHttpFields()
  public boolean org.eclipse.jetty.server.Response.isWritingOrStreaming()
  public org.eclipse.jetty.server.HttpOutput org.eclipse.jetty.server.Response.getHttpOutput()
  public void org.eclipse.jetty.server.Response.resetContent()
  public org.eclipse.jetty.server.HttpChannel org.eclipse.jetty.server.Response.getHttpChannel()
  public org.eclipse.jetty.http.MetaData$Response org.eclipse.jetty.server.Response.getCommittedMetaData()
  public java.util.function.Supplier org.eclipse.jetty.server.Response.getTrailers()
  public void org.eclipse.jetty.server.Response.replaceCookie(org.eclipse.jetty.http.HttpCookie)
  public boolean org.eclipse.jetty.server.Response.isContentComplete(long)
  public void org.eclipse.jetty.server.Response.completeOutput() throws java.io.IOException
  public void org.eclipse.jetty.server.Response.completeOutput(org.eclipse.jetty.util.Callback)
  public void org.eclipse.jetty.server.Response.reopen()
  public void org.eclipse.jetty.server.Response.putHeaders(org.eclipse.jetty.http.HttpContent,long,boolean)
  public static void org.eclipse.jetty.server.Response.putHeaders(javax.servlet.http.HttpServletResponse,org.eclipse.jetty.http.HttpContent,long,boolean)
  public final native void java.lang.Object.wait(long) throws java.lang.InterruptedException
  public final void java.lang.Object.wait(long,int) throws java.lang.InterruptedException
  public final void java.lang.Object.wait() throws java.lang.InterruptedException
  public boolean java.lang.Object.equals(java.lang.Object)
  public native int java.lang.Object.hashCode()
  public final native java.lang.Class java.lang.Object.getClass()
  public final native void java.lang.Object.notify()
  public final native void java.lang.Object.notifyAll()

Notice that the Jetty server didn't use WEB-INF/lib/jakarta.servlet-api-4.0.4.jar ? (It used the 3.1.0 version in the jetty-home container classloader)
That means you cannot reference the new methods in your PR here via the servlet 4.0 API on Jetty 9.4.x
You have to cast to see them.

[joakime]

The goal of this PR is to reach Trailer feature parity with Servlet 4.0 without casting or using Jetty internal classes.
However, in order for this PR to work, you have to cast to the Jetty Response object when working in a webapp.

The existing Jetty 9.4.x Response.setTrailers() exists for embedded users to use with non-servlet APIs (the Handler and Proxy APIs for example), with an interface that was eventually adopted into the Servlet 4.0 spec with some minor changes (eg: HttpFields to Map).

However, a HttpServlet can be used, with Jetty internal classes, but only in Embedded Jetty, and by using the ServletContextHandler (note: not the WebAppContext) This ServletContextHandler is a specialized Handler in Jetty to allow working with the javax.servlet.ServletContext in a way that does not comply with the servlet and war specs) ...

Here's an example of the Servlet in a branch:
See: https://github.com/joakime/use-servlet-4/blob/embedded-servlet-context-handler/src/main/java/demo/DumpServlet.java

        Response baseResponse = Request.getBaseRequest(req).getResponse();
        baseResponse.setTrailers(() ->
        {
            HttpFields trailers = new HttpFields();
            trailers.put("X-Demo", "Foo");
            return trailers;
        });

With the testcase using the embedded ServletContextHandler at ..
https://github.com/joakime/use-servlet-4/blob/embedded-servlet-context-handler/src/test/java/demo/SetTrailersTest.java

Note that this SetTrailersTest does not use servlet annotations, or the WEB-INF/web.xml, or has a webapp initialization phase, or classloader isolation, or support for things like web fragments, or web resources, like a normal WAR or WebAppContext would.

Note that Request.getBaseRequest(HttpServletRequest) will unwrap to the base Jetty Request on it's own (undoing any HttpServlet(Request|Response)Wrappers along the way).
But it only functions outside of a WebAppContext in a fully embedded context that has no servlet spec classloader behaviors.
The raw Jetty Response object can be obtained from the Jetty Request.getResponse() method.
From there, it's trivial to use. But again, this is wholly within the context of an Embedded Scenario, not a Servlet spec scenario.
But if you are going this far, to use embedded Jetty, there's no need for this PR.
But if you don't go this far, and use Jetty from a WAR or WebAppContext, then this PR doesn't serve its stated goal (no casting, no jetty internals).

Of Note: If you use a WAR or WebAppContext, you can also not cast to org.eclipse.jetty.server.Response or org.eclipse.jetty.http.HttpFields as they are hidden from the webapp per spec.
This classloader spec requirement doesn't exist outside of the WebAppContext (such as when using the ServletContextHandler)

[janbartel]

@niloc132 your PR will not work in jetty-9.4 because it requires users to access jetty's Response object: the api trailer methods do not exist < servlet4, and the servlet4 api is not available in jetty-9.4. So we can't accept this PR. Our suggestion for you is that you write a HttpServletResponseWrapper that works only via reflection (no downcasting) that will call through to jetty's Response object when the servlet4 api is not available.

[niloc132]

Sorry for the delay, I got caught up working on Jakarta support (to add jetty 11 and tomcat 10 to the set of versions that the tool I was writing this for needed): grpc/grpc-java#8596

As above, I certainly understand your perspective, disagreeing that it is appropriate to update Jetty 9, and instead focusing on 10 and 11, but my requirements lead me down this road. The linked patch has a class JettyHttpServletResponse which, without reflection (aside from a getClass call, which as above is not quite enough to rigorously solve this) is able to handle this use case, by only casting to jetty's Response if necessary. The tests use embedded jetty, and add javax.servlet-api 4 to the classpath, which then successfully passes all tests.

I likewise recognize that not all users of Jetty use it as an embedded webserver, but I would hazard that only out of curiosity have I ever not deployed jetty as a library embedded in a standalone application. My experience may not be common, but I was first introduced some 10+ years ago to this outstanding product as a handy way to lightly embed Java servlets, and it tends to be the first tool I reach for when serving web content (and as above, always embedded).

At least at a quick test (enable http2/c, enable annotations) I am finding that a deployed war in a non-embedded Jetty can't access some jetty types anyway - I tried to use Here's an example of the Servlet in a branch:
See: https://github.com/joakime/use-servlet-4/blob/embedded-servlet-context-handler/src/main/java/demo/DumpServlet.java with latest Jetty 9.4, but the pom specifies jetty as a compile+runtime dependency, so an extra copy of Jetty is included inside the war (and so the Request.getBaseRequest(...) returns null). This should only be a compile-time dependency, right, so that the webapp classloader holds the only copy of the class? With that dependency changed to scope=provided, this error occurs.

<tr><th>STATUS:</th><td>500</td></tr>
<tr><th>MESSAGE:</th><td>java.lang.NoClassDefFoundError: org/eclipse/jetty/http/HttpFields</td></tr>
<tr><th>SERVLET:</th><td>dump</td></tr>
<tr><th>CAUSED BY:</th><td>java.lang.NoClassDefFoundError: org/eclipse/jetty/http/HttpFields</td></tr>
<tr><th>CAUSED BY:</th><td>java.lang.ClassNotFoundException: org.eclipse.jetty.http.HttpFields</td></tr>
</table>
<h3>Caused by:</h3><pre>java.lang.NoClassDefFoundError: org/eclipse/jetty/http/HttpFields
	at java.base/java.lang.Class.getDeclaredMethods0(Native Method)
	at java.base/java.lang.Class.privateGetDeclaredMethods(Class.java:3166)
	at java.base/java.lang.Class.getDeclaredMethods(Class.java:2309)
	at org.eclipse.jetty.annotations.ResourceAnnotationHandler.doHandle(ResourceAnnotationHandler.java:71)
	at org.eclipse.jetty.annotations.AnnotationIntrospector$AbstractIntrospectableAnnotationHandler.handle(AnnotationIntrospector.java:71)
	at org.eclipse.jetty.annotations.AnnotationIntrospector.introspect(AnnotationIntrospector.java:106)
	at org.eclipse.jetty.annotations.AnnotationDecorator.introspect(AnnotationDecorator.java:62)
	at org.eclipse.jetty.annotations.AnnotationDecorator.decorate(AnnotationDecorator.java:68)
	at org.eclipse.jetty.util.DecoratedObjectFactory.decorate(DecoratedObjectFactory.java:85)
	at org.eclipse.jetty.servlet.ServletContextHandler$Context.createInstance(ServletContextHandler.java:1299)
	at org.eclipse.jetty.server.handler.ContextHandler$StaticContext.createServlet(ContextHandler.java:2898)
	at org.eclipse.jetty.servlet.ServletHolder.newInstance(ServletHolder.java:1202)
	at org.eclipse.jetty.servlet.ServletHolder.initServlet(ServletHolder.java:592)
	at org.eclipse.jetty.servlet.ServletHolder.getServlet(ServletHolder.java:486)
	at org.eclipse.jetty.servlet.ServletHolder.prepare(ServletHolder.java:759)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:545)

Perhaps I missed a step in setting up jetty (I'm unfamiliar with the --add-to-start wiring, as I effectively always use embedded jetty, as above), but this actually seems like the correct behavior, that the child war cannot/shouldn't access the parent classes?

[joakime]

Seems like you are hitting the standard webapp classloader isolation on the Servlet spec.

Server classes are not visible to the webapp.
Server classes cannot be overridden by the webapp.
JVM classes cannot be overridden by the webapp.
Spec / API classes are the only exposure between the Server and WebApp. (and those cannot be overridden by the webapp too)

[niloc132]

I agree - and in embedded situations, it is easy to replace the servlet-api jar with a newer version to support doing this, in cases where it matters. The proposed workaround (or the like) will not work in these cases - reflection could be used, adding some complexity and cost here.

My preference is to support this in what seems to be the best implementation of http2 servlets available, while still maintaining Java 8 support. The grpc-java project actually supports Java 7 as its baseline (for android I assume?), but as the servlet-api 4.0 spec requires Java 8 I have only set the minimum to be Java 8, to better keep with those requirements (as the jump from Java 7 to 8 is much smaller than 8 to 11). Tomcat and Undertow support running in Java 8 with h2, and it was my hope to showcase Jetty as well.

I do not object to you closing this if you aren't interested in supporting this case. Even in that case, I am not looking for support to build this workaround (I think I have the tools to make examples should I or other downstream users need this). I appreciate the help you've offered here, but I am mostly interested in ensuring that Jetty is in front of users as a first-class option when building a gRPC application which needs more functionality than just a bare gRPC server (as netty offers). If that isn't a priority, I can guard the Jetty integration tests with IE11 checks, and ensure the examples guide users to the most compatible other servers.

If there are ways that I can improve the patch such that it might be accepted, please let me know and I will work on them, but if this isn't of interest at all, I understand.

Again, I appreciate the work you've done to help with this issue, and understand that maintaining a project such as Jetty is complex and has many competing considerations.

[joakime]

My preference is to support this in what seems to be the best implementation of http2 servlets available, while still maintaining Java 8 support. The grpc-java project actually supports Java 7 as its baseline (for android I assume?)

Android has supported Java 8 since API 26 (roughly the end of 2017)

Android supports Java 11 now - https://android-developers.googleblog.com/2020/12/announcing-android-gradle-plugin.html

, but as the servlet-api 4.0 spec requires Java 8 I have only set the minimum to be Java 8, to better keep with those requirements (as the jump from Java 7 to 8 is much smaller than 8 to 11).

This is a bad assumption about the Servlet Spec / API jars.

The bytecode level of the Spec / API jars have no relationship with implementation and container requirements.

If you write against Servlet 3.0 (Java 5), you should be
able to run on a Servlet 3.0 container, there's nothing
mandating that the container also be at the same java
minumum support level.

Here's a breakdown of similar decisions in another container.

Tomcat 3.3.x - minimum Java 1.1 - Servlet 2.2 (spec at Java 1.0)
Tomcat 4.1.x - minimum Java 1.3 - Servlet 2.3 (spec at Java 1.1)
Tomcat 5.5.x - minimum Java 1.4 - Servlet 2.4 (spec at Java 1.2)
Tomcat 6.0.x - minimum Java 5 - Servlet 2.5 (spec at Java 5)
Tomcat 7.0.x - minimum Java 7 - Servlet 3.0 (spec at Java 5)
Tomcat 8.0.x - minimum Java 7 - Servlet 3.1 (spec at Java 7)

Take Jakarta EE 10 (due out in a few months).
The minimum stated Java version is Java 17.
All Spec / API jars are being built to Java 11 bytecode, all Implementations must pass TCK on Java 17 (by foundation mandate)

You know that Spring 6 is also going Java 17 minimum version right?

Java 7 public support (meaning access to public, non-paid support contract, releases) ended July 2019. Why haven't you dropped support for Java 7 yet?
Java 8 public support will end on March 2022 (only paid support contract folks get access to it from that point on), that's 5 months from now.

Speaking of HTTP/2, since it pretty much requires TLSv1.2.
Java 7 cannot support TLSv1.2 on the server level (the TLSv1.2 support is client side only), and only Java 1.7u95 or greater has support for the required cipher suites that HTTP/2 needs.
1.7u95 (or later) is also only available for paid support contract folks.

Every release of Java ratchets down the security of SSL/TLS based on industry recommendations.
https://java.com/en/jre-jdk-cryptoroadmap.html
The last public release of Java 8 will likely cease to be viable for SSL/TLS a few short months after Premier support ends and Extended support starts.

[niloc132]

I don't mean to relitigate all of these points, but briefly, I do not use or support Java 7 for any of my projects, and I am not a member of the gRPC team. I am seeking to add servlet support for grpc-java, ideally with Jetty, so I can serve static content, provide gRPC services, and offer filters to handle grpc-web (since browsers don't support enough of http2 to handle grpc proper).

While I don't personally use Java 7 on anything, I am still trying to follow the guidelines of the grpc project when submitting patches to them. I do have projects that are still limited to Java 8, but this a business decision made by others, not a deliberate decision on my part. Those others are the product managers I work with, and the downstream consumers they build for. If I were king, we'd all be on Java 17 already (alas, to my knowledge, only a few maintainers have even offered jdk17 builds, adoptopenjdk is not one of them). Not even being minor royalty in the great world of developers, I instead take requirements (even if I disagree with them) and try to produce correct, interoperable, maintainable code.

(I haven't used Spring on purpose since about 2009.)

[joakime]

The thing is, Jetty 9 is Servlet 3.1, on Java 8.
The attempts in this PR, as they currently stand, just cannot work for the majority of people using Jetty 9 (standalone, spring, docker, cloud providers, etc), this PR is impossible for them to use.

The users that can use the features in this PR, are fully embedded-jetty users, thing is, they don't actually don't need this PR, they can just use the features already in the Request/Response objects directly.
Attempting to make it work via the mutated API this PR provides doesn't help embedded-jetty users, or the other users.

As for making other open source projects support Jetty 9 and Jetty 10 even if they themselves only use Java 8 (or in some cases still on Java 6, staring at you logback) is possible, many projects do just that.
There are several strategies here, separate artifacts for Jetty 9 or Jetty 10, MethodHandle logic, reflection logic, JEP 238 techniques, bootclasspath tricks, etc.
I've even seen open source project support Jetty 9 / 10 on javax.servlet and Jetty 11 on jakarta.servlet at the same time with the same jar (using a combination of the above techniques).
Thing is, all of these techniques assume that the open source project desiring multi-container support implement the container specific logic / behavior, use the container specific apis, etc.

If you don't want any of that, and want grpc with servlet 4, you have to use Jetty 10.

</end-of-facts>

Now to the future.

If you are an open source project using the Servlet spec (or any Java EE or Jakarta EE spec, even things as mundane as javax.sql, or javax.annotations), you will have to migrate to continue to have modern support for your project.
This is happening all around us, no project is immune, grpc is not immune to this, they will have to face these same decisions (continue to support an old spec that has no backward or forward compatibility and an ever decreasing list of supported implementations, or migrate to the new specs, and everything that decision means)

Jetty will continue to support javax.servlet.* namespace, but in an increasingly limited fashion as the years go on. (there's even some debate that we are actually not legally allowed to release updates or fixes in the javax.servlet namespace, so it is essential a legally frozen and unsupported API at this point)

The next release, Jetty 12, based on Jakarta EE 10 (JDK 17), Servlet 6.0 (on jakarta.servlet. namespace), has 2 (different) plans to attempt to continue the support for javax.servlet.*, but we don't yet know what the impact will be with either plan, or how successful (Example: will the removal of dead/deprecated APIs in Servlet 6.0 and related specs cause backward compat problems for old users of Servlet? if they are an old user of a deprecated API, what do we do in to map it to Servlet 6? do we even attempt to map? etc.. lots of questions still up in the air here).

Will Jetty (and Tomcat, and Undertow, and Glassfish, etc) support 5 major versions of the servlet spec for users that don't want to upgrade?

• There are still users stuck on Servlet 2.5 as they have implemented hacks to do things that just don't work on Servlet 3.0 or higher (such as implementing Servlet wrappers for request/response mutation/buffering/compression and the new APIs introduced in Servlet 3.x streams that break their plans)
• 3.x users for users that want Async, with explanations that they cannot use Servlet 4.x because they have 3rd party libraries that haven't updated for Servlet 4.x (this one is a strange excuse)
• 4.x users that want a standardized HTTP/2 push API, and are still stuck on javax.servlet.
• 5.x for users that have adopted jakarta.* namespace, or need Jakarta EE 9 specific features, but haven't made code changes.
• 6.x for users for modern users that need the new feature set that Jakarta EE 10 brings.

That's a big ask for an open source web container.

We've already dropped ...

• Jetty 7.0.x thru 7.6.x (Servlet 2.5 on Java 5)
• Jetty 8.0.x thru 8.1.x (Servlet 3.0 on Java 6)
• Jetty 9.0.x thru 9.2.x (Servlet 3.1 on Java 7)
• Jetty 9.3.x (Servlet 3.1 on Java 8)

(Reminder, Jetty versioning since 1995 is <servlet_support>.<major_version>.<minor_version>)

We have a near term future of ...

• Jetty 9.4.x on Servlet 3.1 on Java 8 (in maintenance mode)
• Jetty 10.0.x on Servlet 4.0 on Java 11 (current mainline development, new features here, even things like HTTP/3)
• Jetty 11.0.x on Servlet 5.0 on Java 11 (current mainline development, new features here too)
• and now Jetty 12.0.x on Servlet 6.0 on Java 17 (next mainline development)

For us, that's a future with 3 mainlines, and 1 maintenance mode branch.
That's not sustainable, so we are putting all of our efforts into a single solution on Jetty 12 to support the range of Servlet versions in isolation to each other (this only became possible recently due to concessions by the Servlet spec related to various spec behaviors that have prevented this concept in the past. Eg: no more cross context dispatch requirement, no more object wrapper identity, etc). But that still means Jetty 12 is Java 17+ minimum.

(I haven't used Spring on purpose since about 2009.)

I also do not use Spring, I find it constricting, and often doing things in the most inefficient way possible under the flag of a consistent API for all.

But ignoring spring, as an open source project, is a bad idea.
They are the java equivalent of the 800-lb gorilla.
The fact that they are going essentially from Servlet 3.1 (on current spring) to Servlet 6.0 (on spring 6) will drag an enormous collection of other non-spring java programs and libraries along.

This is interesting in itself, from a planning point of view.

Servlet 4.x saw barely any adoption, I think due to the slow adoption of HTTP/2, also the most common stated reason for people that did upgrade is the existence of the HTTP/2 push API. (which Jetty 9 championed for in the Servlet spec, and also has an API for in Jetty 9)
Servlet 5.x saw more adoption than Servlet 4.x (and not be a small factor, magnitudes larger, the most common stated reason is that javax.* namespace is a dead-end).
Servlet 6.0 will see more adoption than Servlet 4.x and Servlet 5.x put together purely because of spring 6.

[janbartel]

Hi @niloc132. For all the reasons we've listed in prior comments, we will not be able to accept this PR. I hope this doesn't dissuade you from submitting future PRs, as we are keen to involve the jetty user community.

[atagunov]

Very sad an effort to support GRPC on embedded Jetty 9 has been turned shot down

[joakime]

@atagunov Jetty 9 is now at End of Community Support

• EoCS (End of Community Support) for Jetty 9 - June 2022 #7958

Jetty 12 supports javax.servlet.* namespace while using the ee8 environment layer, which is Servlet 4.0

[atagunov]

That is sad as well

[niloc132]

@atagunov while I think this can be made to work in Jetty 9, there are other improvements that can be found in 10+ that will be important for a rigorous gRPC implementation to have. I have not tried to use Jetty 12 as described, but the servlet support that landed in https://github/grpc/grpc-java should work in Jetty 10, 11, and 12.

None of those options work well for teams that require Java 8 - but at the same time I have to question adding gRPC to any legacy Java 8 project that doesn't intend to update beyond that. That said, feel free to contact me off-list on this if needed - I also understand that there may be additional commercial support available for Jetty (even Jetty 9) from WebTide.