Current builds of Http4s are using Servlet 3.0 as build dependency. It would be nice to bump this to Servlet 4.0 before full move to jakarta namespaces and version 5.0.

Now it should be possible to bump Servlet version.

• Tomcat 9 implements Servlet 4.0 https://tomcat.apache.org/whichversion.html
• Jetty 10 also supports Servlet 4.0 https://www.eclipse.org/jetty/

Also, Jetty 10 kept javax.servlet namespace. So, upgrade should not be a breaking change at least for jetty server part.

If considered too dangerous the change could be skipped for series/0.21.

1. Does this require Servlet 3.1?
2. If so, are we going to continue to support Servlet 3.0 without?

My kneejerk reaction to both is yes.

It would be good if there was a way to adapt or extend the current request conversion which is hidden down in Http4sServlet if you want to extract additional attributes or populate the remote user for example.

Jetty supports Push Promises today for Http2 servers. I was in need of this for a project of my own but it doesn't seem that the current Jetty backend supports this today. Created per a conversation w/ @ChristopherDavenport .

Servlet 6 API is already here, but Jakarta EE 10 isn't out yet, and as far as I know, there are no servers that fully implement it yet. 0.26 is the natural fit for it, but that pins us if we have breaking changes in 0.25, and I expect demand will be super low. If this interests you, ping here, and let's talk.

Comment from @rossabaker
Oh, I see the problem
https://github.com/http4s/http4s/blob/master/servlet/src/main/scala/org/http4s/servlet/syntax/ServletContextSyntax.scala
We're setting the timeout there, but not passing it through as a param. That's bad design.

It should be possible add trailer field support as of Servlet 4.

It's a bit challenging, because there is an isTrailerFieldsReady: Boolean method, but no callback for when it turns false to true. We can start checking on the ReadListener's onAllDataRead, but we have to poll if it's false. Gross.

We suspect there is a performance regression in v0.23.12. This was initially noticed by updating http4s-jetty to 0.23.12 which brings servlet 0.23.13 in transitively.

We've observed a ~15-20% performance hit in a production service (increased CPU to handle the same load). Sorry, this is both pretty unscientific in benchmarking terms and we haven't yet isolated the cause (an exact commit). We are almost entirely certain on the release though.

We've isolated this to the 0.23.12 release of this library by manually bumping the servlet version (and keeping all other dependencies constant). 0.23.11 appears fine, 0.23.12 and 0.23.13 show the reduced performance.

In addition it does not appear that http4s-jetty (or jetty itself) are the cause. Running http4s-jetty 0.23.12 alongside servlet 0.23.13 shows the same performance as http4s-jetty 0.23.11 and servlet 0.23.13.

@TimWSpence is currently looking to isolate the commit!

Complete job name: Build and Test (ubuntu-latest, 2.13.10, temurin@11, rootJVM), on a07f6f0

 ==> X org.http4s.servlet.AsyncHttp4sServletSuite.AsyncHttp4sServlet should not reorder lots of itsy-bitsy chunks  30.005s java.util.concurrent.TimeoutException: Future timed out after [30 seconds]
    at scala.concurrent.impl.Promise$DefaultPromise.tryAwait0(Promise.scala:248)
    at scala.concurrent.impl.Promise$DefaultPromise.result(Promise.scala:261)
    at scala.concurrent.Await$.$anonfun$result$1(package.scala:201)
    at cats.effect.unsafe.WorkerThread.blockOn(WorkerThread.scala:668)
    at scala.concurrent.Await$.result(package.scala:124)
    at munit.internal.PlatformCompat$.$anonfun$waitAtMost$1(PlatformCompat.scala:21)
    at scala.util.Try$.apply(Try.scala:210)
    at munit.internal.PlatformCompat$.waitAtMost(PlatformCompat.scala:21)
    at munit.FunSuite.waitForCompletion(FunSuite.scala:51)
    at munit.FunSuite.$anonfun$test$1(FunSuite.scala:37)
    at munit.MUnitRunner.$anonfun$runTestBody$1(MUnitRunner.scala:296)

AsyncHttp4sServlet from a discussion with Ross seems to be missing serviceErrorHandler. Not sure if it should be included or not, but I added a ticket just in case.

Here is what methods are included currently.

AsyncHttp4sServlet
          .builder(httpApp, dispatcher)
          .withAsyncTimeout(builder.asyncTimeout)
          .withChunkSize(chunkSize)
          .build

Proposed support matrix:

Jetty 10 requires Java 11. We don't need to go to Java 11 in this repo until Servlet 6, but we'd have to replace Jetty in our tests. The forthcoming http4s-jetty will have to go 11.

Servlet 4 isn't really all that interesting, but keeping a minor version as a placeholder just in case seems prudent. I could be talked into lazily materializing that branch.

I am not solving the problem of cross-building http4s versions for servlet versions. I care deeply that we continue to support servlets it in some form, but there's not nearly enough demand to support it in cartesian product form.

I'm devoted to keeping servlets working, but happy to have a second set of eyes.

I noticed some odd behavior when sending a multipart request to an http4s service wrapped in a servlet.

object MyService {
  def apply(): HttpRoutes[IO] = HttpRoutes.of {
    case request =>
       request.decode[Multipart[IO]] { multipart =>
        Ok(s"Parts: ${multipart.parts.length}")
      }
  }
}

Requests work as expected when using the service without a Servlet.

curl -F "content=@/image.jpg" -v http://localhost:8080/
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8080 (#0)
> POST / HTTP/1.1
> Host: 127.0.0.1:8080
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Length: 4426
> Expect: 100-continue
> Content-Type: multipart/form-data; boundary=------------------------d3eedf51ac8c72ec
> 
* Done waiting for 100-continue
< HTTP/1.1 200 OK
< Content-Type: text/plain; charset=UTF-8
< Date: Fri, 04 Jan 2019 11:19:42 GMT
< Content-Length: 8
< 
* Connection #0 to host 127.0.0.1 left intact
Parts:  1

But doing the same with a (Blocking) Http4sServlet yields an odd response.

curl -F "content=@/image.jpg" -v http://localhost:8080/
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8080 (#0)
> POST /test HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Length: 4426
> Expect: 100-continue
> Content-Type: multipart/form-data; boundary=------------------------2238d13e7d3aa3a6
> 
< HTTP/1.1 100 Continue
< HTTP/1.1 200 OK
< cache-control: no-cache
< expires: Mon, 01 Jan 1990 00:00:00 GMT
< Content-Type: text/html
< Content-Length: 0
< Server: Development/2.0
< Date: Fri, 04 Jan 2019 11:22:18 GMT
< 
* Connection #0 to host localhost left intact

After digging a bit deeper I realized that the servlet request processing is actually stuck here. I played around with it and managed to get it to work somehow, but without understanding why it's failing in the first place. In order to provide a proper patch a better understanding would be necessary though. I assume that the AsyncHttp4sServlet suffers from the same issue, but could not verify that yet.

I'm not very familiar with the HTTP-servlet standard, so perhaps http4s is technically in-the-right in assuming the ServletInputStream has not yet been read from. However, it would certainly make the library more user-friendly (and have saved me hours) if the BlockingServletIo raised something like an IllegalStateException -- including a pertinent error message -- if the input-stream's isFinished method returns true out-the-gate.

In my case, I had a javax.servlet.Filter registered that was calling getParameterMap on the HttpServletRequest (to do some primitive malicious-request rejection), before passing off the request object off to the http4s stack... It looks like this was invoking Jetty's implementation of getParameterMap, which reads "POST" parameters (so to speak) from the request body.

The result is that I would end up with an empty Map when doing something like req.decode[UrlForm] { f => val params = f.values; /* ... */ } and no sign of what had gone wrong.