Router handling prefix incorrectly when using Jetty server

[talbotja]

If I use a Router where the prefix includes a forward slash, I would expect the whole prefix to be stripped from the request before matching on the inner routes. More specifically, if I have routes like this:

org.http4s.server.Router(
  "pref-a/pref-b" -> myRoutes
)

And I make a request to pref-a/pref-b/my-resource, I would expect to be able to match Root / "my-resource" within myRoutes.

This appears to work as expected if I use blaze as the server, but if I use jetty it only removes the first segment of the prefix, meaning that I have to match Root / "pref-b" / "my-resource".

Not sure how good a job I did explaining that, so I have reproduced it in this repo. It spins up jetty on 8080 and blaze on 8081 serving the same routes. You can see that we get the expected behaviour from blaze but not from jetty.

[talbotja]

FWIW I did a tiny bit of digging and I wonder if this method in http4s-servlet could be a problem? It looks like Router ends up combining that -1 with the caret it gets from the prefix it is provided (which I guess would be 2 in this case), meaning that the caret is ultimately one segment too early?

I have zero experience with the internals of http4s, so apologies if that's way off 😬

[fthomas]

@talbotja I think you're hitting the same bug as I did in http4s/http4s#5100. And I agree with you that getPathInfoIndex should probably not return -1 if it couldn't find a split. Returning None in this case at least fixed the failing test in http4s/http4s#5100.

[talbotja]

@fthomas ah yes thanks, that does look like exactly the same thing. I confirmed the same issue exists using tomcat as the server too, which makes sense given that that's the other use of Http4sServlet (I think?).

I tried out your proposed fix locally and it does fix the issue I'm seeing 👍

[fthomas]

@talbotja I currently don't have time to bring http4s/http4s#5100 in a mergeable state. So if you're inclined to fix this issue yourself, fell free to go ahead.

[talbotja]

Apologies for the delay on this, but I actually have some time I could spend on this this week.

What would need to be done in order for something like the proposed change in http4s/http4s#5100 to be mergeable? Some tidying up of the code in Http4sServlet.scala, and perhaps a bit more test coverage? Anything else? I don't have experience contributing to http4s before but I'm happy to have a go at setting up my own PR here.

[armanbilge]

Fantastic! It looks like @hamnis had some ideas what to do in http4s/http4s#5100.

[xnejp03]

Any movement on this? I think we're hitting the same issue with a setup like this:

val r = Router(
  "/ping" -> PingRoutes.routes
)

class PingRoutes extends Http4sDsl[IO] {
  def routes: HttpRoutes[IO] = {
    HttpRoutes.of[IO] {
      case GET -> Root => Ok("pong")        
    }
}

GET /ping returns 404. This use to work in 0.21.x.
It currently makes the Router pretty much pointless (also using Jetty)

[armanbilge]

@xnejp03 I don't remember seeing any PRs relating to this. Would be great if someone can pick it up.

[xnejp03]

Is this not one? http4s/http4s#5100

[armanbilge]

Yes, sorry I should clarify: I haven't seen any PRs attempting to fix this :) that PR says:

This PR is not intended to be merged as-is but a reproduction of a possible bug

There are some pointers in the comments that may be useful.

Edit: nope, I'm wrong again, apologies. I guess that PR does propose a fix too :) seems like it needed additional investigation though.

[RafalSumislawski]

I can have a look into this problem tomorrow.

[RafalSumislawski]

Well... this is a rabbit hole. The http4s/http4s#5100 patches up the obvious problem, but it doesn't address the underlaying issue of some quirky edge-cases in Uri that lead to this problem. I'm working on a solution. I'll keep you updated.