Allow handling of HTTP CONNECT in WebFlux

[GuyLewin]

I'm trying to simulate an CONNECT HTTP request using MockServerHttpRequest, but it only receives HttpMethod as valid methods (not strings).
For some reason CONNECT isn't in the HttpMethod enum.
I don't mind creating a PR for that, just want to make sure it's not intentional.

[rstoyanchev]

HttpMethod and also RequestMethod were created long before WebFlux and prior to that CONNECT wasn't and still isn't applicable to Spring MVC. It's not supported in the Servlet API and while it's possible to override HttpServlet#service in order handle it, it wouldn't be very useful to handle with blocking I/O.

Your use case is a WebFlux application that handles CONNECT as a proxy? We could add it to HttpMethod and RequestMethod now that there are valid ways to use it.

[GuyLewin]

Thanks @rstoyanchev. My use case is a bit rare I would assume - I want to handle CONNECT requests (as a forward proxy server) and parse some things out of them. In my scenario - under the CONNECT header there's a bunch of HTTP requests (connection keepalived), so to Spring it should look like a normal HTTP/1.1 session with a CONNECT method as the first request.

The other major issue that I found is that when a CONNECT HttpRequest from Reactive Netty is wrapped with ServerHttpRequest - there's a bug in parsing the URI (since the part that is usually the "path" is now a host with colon and without a leading slash) and the path isn't parsed correctly - it's an empty string. Should I open a separate ticket for that?

[rstoyanchev]

I changed the title to reflect we may need to do a few things. Do you have a sample app handy for the URI parsing issue or if you could, please provide details on the steps you follow

[rstoyanchev]

On closer look, WebFlux is for handling HTTP requests but in HTTP CONNECT only the initial request looks like a regular HTTP request-response. After that you need access to the underlying TCP connection to tunnel subsequent requests. WebFlux is not the right level to do this.

Netty has support for proxying with HTTP CONNECT so I suggest to try with Reactor Netty.

[GuyLewin]

@rstoyanchev sorry for the (very) late comment - Netty DOES support HTTP CONNECT (https://netty.io/4.0/api/io/netty/handler/codec/http/HttpMethod.html#CONNECT).
I believe the Spring WebFlux implementation should support what Netty does. Right now it's impossible to simulate HTTP CONNECT requests using MockServerHttpRequest because it only accepts Spring's HttpMethod enum which doesn't include CONNECT.
Even simply adding CONNECT there would solve most of my problem.

[rstoyanchev]

@GuyLewin we can certainly make it possible to specify a custom HTTP method, via a String, on MockServerHttpRequest, is that all you're looking for?

As for the HttpMethod and RequestMethod enums, I don't think it makes sense to add it there because neither Spring MVC nor WeFlux are in a good position to handle HTTP CONNECT for which it is best to have access to the underlying TCP connection.

[GuyLewin]

@rstoyanchev fixing MockServerHttpRequest will already be enough for my code :)

I totally understand there's no underlaying TCP connection support so there's no point to add support, but I still find it weird that currently if WebFlux receives a CONNECT method it's just parsed completely wrong (the path parameter is broken). Perhaps we should fix that as well, but still not add support for CONNECT method in the enum, just so read-only parsing of CONNECT will not be broken?

[sbrannen]

@GuyLewin, would you mind opening a new issue to request the custom HTTP method support in MockServerHttpRequest?

[rstoyanchev]

Yes allowing a custom HTTP method on MockServerHttpRequest is generally useful. Please open a more focused issue for that and we'll get it done.

but I still find it weird that currently if WebFlux receives a CONNECT method it's just parsed completely wrong (the path parameter is broken)

I was not able to reproduce this part. I did experiment with adding CONNECT to HttpMethod and RequestMethod I was able to successfully invoke a controller method. Depending on what the issue is we could fix it but for that I would need some sample.

[GuyLewin]

@rstoyanchev thanks!
The problem with CONNECT wasn't a controller method, it's the ServerHttpRequest object that is created.
In this example I received a HTTP CONNECT request into my Spring Cloud Gateway that looks as such:

CONNECT somehost:80 HTTP/1.1
Host: somehost:80
...more headers here...

And this is how it's parsed in Spring Cloud Gateway:

Notice the nettyRequest knows about the uri inside being the host since it was able to parse it correctly.
Spring's path is blank and the actual CONNECT destination was blended into uri (because of Host header).

[rstoyanchev]

The problem with CONNECT wasn't a controller method, it's the ServerHttpRequest object that is created.

Yes I got that but one implies the other doesn't it? If a request isn't be parsed correctly, it also isn't going to map to a controller. In my simple WebFlux server example, I debugged and examined the request details, and all looked good. There is something less obvious going on like forwarded headers, or something about the client.

If you can provide an isolated sample and the issue seems relevant to supported methods, or at least seams a reasonable way to tighten the code, I see no issue with fixing it but beyond I don't see what the end goal here is for an HTTP method that is not supported to begin with.

[GuyLewin]

@rstoyanchev I understand. I looked at the code over at org/springframework/http/server/reactive/ReactorServerHttpRequest.class and the problem is because of

private static URI initUri(HttpServerRequest request) throws URISyntaxException {
    Assert.notNull(request, "HttpServerRequest must not be null");
    return new URI(resolveBaseUrl(request).toString() + resolveRequestUri(request));
  }

It appears Spring's ReactorServerHttpRequest builds a Java URI from the value of Host header + the resource identifier of the request.
According to HTTP/1.1 RFC section 5.3 - this is just 1 of 4 different resource identifier types (Spring supports origin-form, CONNECT uses authority-form but there are 2 others described in the RFC).

I understand WebFlux isn't built to support these other forms and that's why it's tightly coupled, but it seems strange to me if someone wants to create something a bit less straight-forward (like myself) they can't overcome these issues. I feel like Spring should at least support getting the original resource identifier in addition to getting the parsed URI in order to support these corner cases.
What do you think?