jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.85k stars 1.91k forks source link

Lockup processing POST request body with Jetty 12.0.1 using http/2 #10513

Open ghost opened 1 year ago

ghost commented 1 year ago

Jetty version(s) 12.0.1

Jetty Environment ee10 / http/2

Java version/vendor (use: java -version) OpenJDK Runtime Environment (Red_Hat-21.0.0.0.35-1.rolling.fc38) (build 21+35) OpenJDK 64-Bit Server VM (Red_Hat-21.0.0.0.35-1.rolling.fc38) (build 21+35, mixed mode, sharing)

OS type/version Fedora 38 Workstation

Description This application sets up an embedded Jetty 12.0.1 instance with one servlet class TestServlet1. The doPost method simply tries to read-and-discard the content of the body before sending a short response. Posting a sufficiently large file to the application will result in a lockup. The curl client will block trying to send more data while the application blocks trying to read. The point where th size of the POST body is printed to stderr is then never reached.

A slight modification of the code makes it use TestServlet2 with a doPost that processes the input expecting a ZIP-formatted body by reading and discarding entries and printing the name:size of each entry to stderr. Posting a sufficiently large ZIP file to it shows that initial entries are processed corrrectly then it locks up the same way as with TestServlet1.

Small POST requests seem to work without issues. GET requests seem to work fine also and our application ported from Jetty 11 to Jetty 12 works mostly fine except for large POST requests.

How to reproduce? See the above description. I am sorry that this is Scala code instead of Java. I currently have no Java development environment setup.

Compile/runtime dependencies specified in build.sbt, some of which redundant for the test code

libraryDependencies += "org.eclipse.jetty" % "jetty-core" % "12.0.1"
libraryDependencies += "org.eclipse.jetty" % "jetty-io" % "12.0.1"
libraryDependencies += "org.eclipse.jetty" % "jetty-server" % "12.0.1"
libraryDependencies += "org.eclipse.jetty" % "jetty-alpn-server" % "12.0.1"
libraryDependencies += "org.eclipse.jetty" % "jetty-session" % "12.0.1"
libraryDependencies += "org.eclipse.jetty.ee10" % "jetty-ee10-servlet" % "12.0.1"
libraryDependencies += "org.eclipse.jetty.websocket" % "jetty-websocket-jetty-api" % "12.0.1"
libraryDependencies += "org.eclipse.jetty.websocket" % "jetty-websocket-jetty-server" % "12.0.1"
libraryDependencies += "org.eclipse.jetty.ee10.websocket" % "jetty-ee10-websocket-jetty-server" % "12.0.1"
libraryDependencies += "org.eclipse.jetty.http2" % "jetty-http2-server" % "12.0.1"
libraryDependencies += "org.eclipse.jetty.http3" % "jetty-http3-server" % "12.0.1"

Scala code

package nl.idfix.util.test

import java.io._
import java.util.zip._

import jakarta.servlet._
import jakarta.servlet.http._

import org.eclipse.jetty.alpn._
import org.eclipse.jetty.alpn.server._
import org.eclipse.jetty.http2._
import org.eclipse.jetty.http2.server._
import org.eclipse.jetty.http3._
import org.eclipse.jetty.http3.server._
import org.eclipse.jetty.util.resource._
import org.eclipse.jetty.security._
import org.eclipse.jetty.server._
import org.eclipse.jetty.server.handler.{ ContextHandler, ResourceHandler, DefaultHandler, ContextHandlerCollection }
import org.eclipse.jetty.ee10.servlet._
import org.eclipse.jetty.session.{ DefaultSessionIdManager, HouseKeeper }
import org.eclipse.jetty.util.security._
import org.eclipse.jetty.util.ssl._
import org.eclipse.jetty.util.thread._
import org.eclipse.jetty.server._
import org.eclipse.jetty.websocket.api._
import org.eclipse.jetty.ee10.websocket.server._
import org.eclipse.jetty.ee10.websocket.server.config._
import org.eclipse.jetty.http.HttpFields

class NullOutputStream extends OutputStream
{
    def write(byte : Int) = ()
    override def write(buff : Array[Byte]) = ()
    override def write(buff : Array[Byte],start : Int,count : Int) = ()
}

class CountingOutputStream(wrapped : OutputStream) extends OutputStream
{
    private var written : Long = 0

    def bytesWritten = written

    def write(byte : Int)
    {
        wrapped.write(byte)
        written += 1
    }

    override def write(buffer : Array[Byte],offset : Int,bytes : Int)
    {
        wrapped.write(buffer,offset,bytes)
        written += bytes
    }

    override def write(buffer : Array[Byte])
    {
        wrapped.write(buffer)
        written += buffer.length
    }

    override def flush = wrapped.flush
    override def close = wrapped.close
}

class TestServlet1 extends HttpServlet
{
    override def doPost(request : HttpServletRequest,response : HttpServletResponse)
    {
        val in = request.getInputStream
        val out = new CountingOutputStream(new NullOutputStream)
        in.transferTo(out)
        in.close
        out.close
        System.err.println("BYTE COUNT:" + out.bytesWritten)
        response.setHeader("Content-Type","text/plain")
        val writer = response.getWriter
        writer.println("Thank you")
        writer.close
    }
}

class TestServlet2 extends HttpServlet
{
    override def doPost(request : HttpServletRequest,response : HttpServletResponse)
    {
        val in = request.getInputStream
        val zip = new ZipInputStream(in)

        var entry = zip.getNextEntry
        while (entry != null)
        {
            val out = new CountingOutputStream(new NullOutputStream)
            zip.transferTo(out)
            out.close
            System.err.println(entry.getName + ":" + out.bytesWritten)
            entry = zip.getNextEntry
        }
        zip.close
        val writer = response.getWriter
        writer.println("Thank you")
        writer.close
    }
}

object JettyTest
{
    def main(args: Array[String])
    {
        val pool = new QueuedThreadPool
        val server = new Server(pool)

        /* Create connector */
        val keyStore = "abc.jks"
        val keyPassword = "secret"
        val sslContextFactory = new SslContextFactory.Server
        sslContextFactory.setEndpointIdentificationAlgorithm("HTTPS")
        sslContextFactory.setKeyStorePath(keyStore)
        sslContextFactory.setKeyStorePassword(keyPassword)
        sslContextFactory.setKeyManagerPassword(keyPassword)
        sslContextFactory.setKeyStoreType("JKS")
        val httpsConfig = new HttpConfiguration
        httpsConfig.setSecureScheme("https")
        httpsConfig.setSecurePort(8443)
        val http1 = new HttpConnectionFactory(httpsConfig)
        val http2 = new HTTP2ServerConnectionFactory(httpsConfig)
        val alpn = new ALPNServerConnectionFactory
        alpn.setDefaultProtocol(http1.getProtocol)
        val ssl = new SslConnectionFactory(sslContextFactory,alpn.getProtocol)
        val http2Connector = new ServerConnector(server,ssl,alpn,http2,http1)
        http2Connector.setHost("0.0.0.0")
        http2Connector.setPort(8443)
        http2Connector.setIdleTimeout(1800000L)
        server.addConnector(http2Connector)

        /* Create Servlet context */

        val servletContext = new ServletContextHandler
        servletContext.setContextPath("/")
        val holder = new ServletHolder(classOf[TestServlet1])
        servletContext.addServlet(holder,"/*")

        /* Setup server */
        val ctxHandler = new ContextHandlerCollection
        ctxHandler.setHandlers(servletContext)

        server.setHandler(ctxHandler)
        server.start
        server.join
    }
}
joakime commented 10 months ago

Also note that Jetty's GzipHandler can uncompress request body content, you don't need to add a hack to do it yourself.

You just need to specify an inflate buffer size on the GzipHandler for it to start to be used. (set it to 8192 and you'll be good)

GzipHandler gzipHandler = new GzipHandler();
gzipHandler.setInflateBufferSize(8192); // enable request body inflation (aka decompression)
gregw commented 10 months ago

But anything you want me to try/test I will try to fit in.

@silviobierman I don't think we have found a smoking gun for these problems, but we have found a number of issues that can definitely be improved. We will get fixes for those merged in the next few days and then ask you to try again.

gregw commented 10 months ago

Also note that Jetty's GzipHandler can uncompress request body content, you don't need to add a hack to do it yourself.

@joakime I think the issue is that the request is multi-part with a single part that is a compressed file. So the jetty GzipHandler will not see it, as we do not decompress within the parts.

joakime commented 10 months ago

Also note that Jetty's GzipHandler can uncompress request body content, you don't need to add a hack to do it yourself.

@joakime I think the issue is that the request is multi-part with a single part that is a compressed file. So the jetty GzipHandler will not see it, as we do not decompress within the parts.

A prior comment does not indicate multipart is in use - https://github.com/jetty/jetty.project/issues/10513#issuecomment-1837594354

gregw commented 10 months ago

My understanding is that the zipped content can be uploaded either by multipart form content, or as a "plain ZIP upload". However, the problem "described is with a JavaScript FormData originated FORM POST with a single part that contains ZIP content. No unzipping by Jetty" So the problematic case is when our GzipHandler cannot be used.

The errors seen suggest some corruption of the part, which could be the single byte read issue that @lorban is fixing. The hangs could be the fact that we are not resetting the h2 stream with a NO_ERROR reset, so end up waiting for content that will not be sent.

ghost commented 10 months ago

@joakime @gregw : The ZIP-archives are generated by the reverse process in the GET-handler. All tests I have done revealed that reading all its entries fully in the POST-handler left 0 bytes at the end of the request/part. So either as the sole content of a POST or inside a multi-part request with a ZIP-archive part the application processes the ZIP-archive fully reading all bytes of all entries.

And this is not about transfer encoding with gzip but simply content types that happen to be (sometimes nested meaning ZIP inside ZIP) ZIP-archives which are processed by the application in several different ways depending on context and content.

Correct me if I am wrong but I was under the assumption that it is the containers responsibility to keep up the notion of separate independent requests meaning it will drop unprocessed bytes after something like a synchronous doPost returns or throws and not let them leak through into subsequent requests.

ghost commented 10 months ago

There is a common trait in all these cases: the server needs considerable time to process the data and does not buffer it to finish the request early. The back pressure results in a client (in my ZIP-archive test case being the browser) waiting sometimes several minutes up to even 45-60 minutes in extreme cases for the server to accept all the data. My current test case is a 503Mb ZIP that takes about a minute to process on localhost which is my laptop with an NVME SSD. Practical cases of restoring full backups go upto 60-70Gb ZIP-archives with a curl client (no multipart) which may take close to an hour.

The CSV feeds that went wrong for the Expect:100-continue clients (ie. .NET clients) cases take much less time in the range of 10-30 seconds.

gregw commented 10 months ago

I think we are looking at several problems here. We will soon have some new images for you to test. stand by...