Skip to content

Commit

Permalink
Support for "raw protocol" string, that contains protocol and its ver…
Browse files Browse the repository at this point in the history
…sion in HttpPrologue (#5575)
  • Loading branch information
tomas-langer authored Dec 1, 2022
1 parent e4f9f7c commit c8fea30
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 24 deletions.
31 changes: 25 additions & 6 deletions common/http/src/main/java/io/helidon/common/http/HttpPrologue.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* A prologue of an HTTP protocol.
*/
public class HttpPrologue {
private final String rawProtocol;
private final String protocol;
private final String protocolVersion;
private final Http.Method method;
Expand All @@ -45,12 +46,14 @@ public class HttpPrologue {
* @param rawFragment fragment as received over the network; may be {@code null} when the prologue does not contain a
* fragment
*/
private HttpPrologue(String protocol,
private HttpPrologue(String rawProtocol,
String protocol,
String protocolVersion,
Http.Method method,
UriPath path,
String rawQuery,
String rawFragment) {
this.rawProtocol = rawProtocol;
this.protocol = protocol;
this.protocolVersion = protocolVersion;
this.method = method;
Expand All @@ -59,12 +62,14 @@ private HttpPrologue(String protocol,
this.rawFragment = rawFragment;
}

private HttpPrologue(String protocol,
private HttpPrologue(String rawProtocol,
String protocol,
String protocolVersion,
Http.Method httpMethod,
UriPath uriPath,
UriQuery uriQuery,
UriFragment uriFragment) {
this.rawProtocol = rawProtocol;
this.protocol = protocol;
this.protocolVersion = protocolVersion;
this.method = httpMethod;
Expand All @@ -79,14 +84,16 @@ private HttpPrologue(String protocol,
/**
* Create a new prologue.
*
* @param rawProtocol raw protocol string (full HTTP/1.1 or similar)
* @param protocol protocol
* @param protocolVersion protocol version
* @param httpMethod HTTP Method
* @param unresolvedPath unresolved path
* @param validatePath whether to validate path (that it contains only allowed characters)
* @return a new prologue
*/
public static HttpPrologue create(String protocol,
public static HttpPrologue create(String rawProtocol,
String protocol,
String protocolVersion,
Http.Method httpMethod,
String unresolvedPath,
Expand Down Expand Up @@ -117,7 +124,8 @@ public static HttpPrologue create(String protocol,
uriPath.validate();
}

return new HttpPrologue(protocol,
return new HttpPrologue(rawProtocol,
protocol,
protocolVersion,
httpMethod,
uriPath,
Expand All @@ -128,6 +136,7 @@ public static HttpPrologue create(String protocol,
/**
* Create a new prologue with decoded values.
*
* @param rawProtocol raw protocol string (full HTTP/1.1 or similar)
* @param protocol protocol
* @param protocolVersion protocol version
* @param httpMethod HTTP Method
Expand All @@ -136,13 +145,23 @@ public static HttpPrologue create(String protocol,
* @param uriFragment resolved fragment
* @return a new prologue
*/
public static HttpPrologue create(String protocol,
public static HttpPrologue create(String rawProtocol,
String protocol,
String protocolVersion,
Http.Method httpMethod,
UriPath uriPath,
UriQuery uriQuery,
UriFragment uriFragment) {
return new HttpPrologue(protocol, protocolVersion, httpMethod, uriPath, uriQuery, uriFragment);
return new HttpPrologue(rawProtocol, protocol, protocolVersion, httpMethod, uriPath, uriQuery, uriFragment);
}

/**
* Raw protocol, should be {@code HTTP/1.1} or {@code HTTP/2} in most cases.
*
* @return protocol
*/
public String rawProtocol() {
return rawProtocol;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
class HttpPrologueTest {
@Test
void testPrologueWithAll() {
HttpPrologue prologue = HttpPrologue.create("HTTP",
HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
"HTTP",
"1.1",
Http.Method.GET,
"/admin;a=b/list;c=d;e=f?first=second#fragment",
true);

assertThat(prologue.rawProtocol(), is("HTTP/1.1"));
assertThat(prologue.protocol(), is("HTTP"));
assertThat(prologue.protocolVersion(), is("1.1"));
assertThat(prologue.method(), is(Http.Method.GET));
Expand All @@ -43,12 +45,14 @@ void testPrologueWithAll() {
void testPrologueEncodedPath() {
String path = "/one/two?a=b%26c=d&e=f&e=g&h=x%63%23e%3c#a%20frag%23ment";

HttpPrologue prologue = HttpPrologue.create("HTTP",
HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
"HTTP",
"1.1",
Http.Method.GET,
path,
true);

assertThat(prologue.rawProtocol(), is("HTTP/1.1"));
assertThat(prologue.protocol(), is("HTTP"));
assertThat(prologue.protocolVersion(), is("1.1"));
assertThat(prologue.method(), is(Http.Method.GET));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
* A single connection serves multiple streams.
*/
public class Http2Connection implements ServerConnection {
static final String FULL_PROTOCOL = "HTTP/2.0";
static final String PROTOCOL = "HTTP";
static final String PROTOCOL_VERSION = "2.0";

Expand Down Expand Up @@ -494,7 +495,8 @@ private void doHeaders() {
// todo configure path validation
String path = headers.path();
Http.Method method = headers.method();
HttpPrologue httpPrologue = HttpPrologue.create(PROTOCOL,
HttpPrologue httpPrologue = HttpPrologue.create(FULL_PROTOCOL,
PROTOCOL,
PROTOCOL_VERSION,
method,
path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ public ServerConnection upgrade(ConnectionContext ctx,
it -> http2Headers.authority(it.value()));
http2Headers.scheme("http"); // TODO need to get if https (ctx)?

HttpPrologue newPrologue = HttpPrologue.create(prologue.protocol(),
HttpPrologue newPrologue = HttpPrologue.create(Http2Connection.FULL_PROTOCOL,
prologue.protocol(),
Http2Connection.PROTOCOL_VERSION,
prologue.method(),
prologue.uriPath(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ void testHelidonFormat() {
PeerInfo pi = mock(PeerInfo.class);
when(pi.host()).thenReturn(REMOTE_IP);
when(request.remotePeer()).thenReturn(pi);
HttpPrologue prologue = HttpPrologue.create("HTTP",
HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
"HTTP",
"1.1",
Http.Method.PUT,
UriPath.create(PATH),
Expand Down Expand Up @@ -103,7 +104,8 @@ void testCommonFormat() {
PeerInfo pi = mock(PeerInfo.class);
when(pi.host()).thenReturn(REMOTE_IP);
when(request.remotePeer()).thenReturn(pi);
HttpPrologue prologue = HttpPrologue.create("HTTP",
HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
"HTTP",
"1.1",
Http.Method.PUT,
UriPath.create(PATH),
Expand Down Expand Up @@ -142,7 +144,8 @@ void testCustomFormat() {

RoutingRequest request = mock(RoutingRequest.class);
PeerInfo pi = mock(PeerInfo.class);
HttpPrologue prologue = HttpPrologue.create("HTTP",
HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
"HTTP",
"1.1",
Http.Method.PUT,
UriPath.create(PATH),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ void testClasspathFromInMemory() throws IOException, URISyntaxException {

ServerRequest req = mock(ServerRequest.class);
when(req.headers()).thenReturn(ServerRequestHeaders.create());
when(req.prologue()).thenReturn(HttpPrologue.create("http", "1.1", Http.Method.GET, "/favicon.ico", false));
when(req.prologue()).thenReturn(HttpPrologue.create("http/1.1",
"http",
"1.1",
Http.Method.GET,
"/favicon.ico",
false));

ServerResponse res = mock(ServerResponse.class);
when(res.headers()).thenReturn(responseHeaders);
Expand All @@ -106,7 +111,7 @@ void testClasspathCacheFound() throws IOException, URISyntaxException {

ServerRequest req = mock(ServerRequest.class);
when(req.headers()).thenReturn(ServerRequestHeaders.create());
when(req.prologue()).thenReturn(HttpPrologue.create("http", "1.1", Http.Method.GET, "/resource.txt", false));
when(req.prologue()).thenReturn(HttpPrologue.create("http/1.1", "http", "1.1", Http.Method.GET, "/resource.txt", false));

ServerResponse res = mock(ServerResponse.class);
when(res.headers()).thenReturn(responseHeaders);
Expand Down Expand Up @@ -138,7 +143,7 @@ void testClasspathCacheRedirectFound() throws IOException, URISyntaxException {

ServerRequest req = mock(ServerRequest.class);
when(req.headers()).thenReturn(ServerRequestHeaders.create());
when(req.prologue()).thenReturn(HttpPrologue.create("http", "1.1", Http.Method.GET, "/nested", false));
when(req.prologue()).thenReturn(HttpPrologue.create("http/1.1", "http", "1.1", Http.Method.GET, "/nested", false));
when(req.query()).thenReturn(UriQuery.empty());

ServerResponse res = mock(ServerResponse.class);
Expand Down Expand Up @@ -177,7 +182,12 @@ void testFsFromInMemory() throws IOException {

ServerRequest req = mock(ServerRequest.class);
when(req.headers()).thenReturn(ServerRequestHeaders.create());
when(req.prologue()).thenReturn(HttpPrologue.create("http", "1.1", Http.Method.GET, "nested/resource.txt", false));
when(req.prologue()).thenReturn(HttpPrologue.create("http/1.1",
"http",
"1.1",
Http.Method.GET,
"nested/resource.txt",
false));

ServerResponse res = mock(ServerResponse.class);
when(res.headers()).thenReturn(responseHeaders);
Expand All @@ -196,7 +206,7 @@ void testFsCacheFound() throws IOException {

ServerRequest req = mock(ServerRequest.class);
when(req.headers()).thenReturn(ServerRequestHeaders.create());
when(req.prologue()).thenReturn(HttpPrologue.create("http", "1.1", Http.Method.GET, "/resource.txt", false));
when(req.prologue()).thenReturn(HttpPrologue.create("http/1.1", "http", "1.1", Http.Method.GET, "/resource.txt", false));

ServerResponse res = mock(ServerResponse.class);
when(res.headers()).thenReturn(responseHeaders);
Expand Down Expand Up @@ -228,7 +238,7 @@ void testFsCacheRedirectFound() throws IOException {

ServerRequest req = mock(ServerRequest.class);
when(req.headers()).thenReturn(ServerRequestHeaders.create());
when(req.prologue()).thenReturn(HttpPrologue.create("http", "1.1", Http.Method.GET, "/nested", false));
when(req.prologue()).thenReturn(HttpPrologue.create("http/1.1", "http", "1.1", Http.Method.GET, "/nested", false));
when(req.query()).thenReturn(UriQuery.empty());

ServerResponse res = mock(ServerResponse.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ private static void assertHttpException(Runnable runnable, Http.Status status) {

private ServerRequest mockRequestWithPath(Http.Method method, String path) {
UriPath uriPath = UriPath.create(path);
HttpPrologue prologue = HttpPrologue.create("HTTP",
HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
"HTTP",
"1.1",
method,
uriPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ class RouteCrawler {
this.parent = parent;

HttpPrologue prologue = request.prologue();
this.prologue = HttpPrologue.create(prologue.protocol(),
this.prologue = HttpPrologue.create(prologue.rawProtocol(),
prologue.protocol(),
prologue.protocolVersion(),
prologue.method(),
child,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ public boolean shouldReroute() {

@Override
public HttpPrologue reroutePrologue(HttpPrologue prologue) {
return HttpPrologue.create(prologue.protocol(),
return HttpPrologue.create(prologue.rawProtocol(),
prologue.protocol(),
prologue.protocolVersion(),
prologue.method(),
UriPath.create(reroutePath),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ private HttpPrologue doRead() {
}

try {
return HttpPrologue.create("HTTP",
return HttpPrologue.create(protocol,
"HTTP",
"1.1",
method,
path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ private void testNoHandler(ErrorHandlers handlers, Exception e, String message)
ServerResponse res = mock(ServerResponse.class);
ConnectionContext ctx = mock(ConnectionContext.class);

when(req.prologue()).thenReturn(HttpPrologue.create("http",
when(req.prologue()).thenReturn(HttpPrologue.create("http/1.0",
"http",
"1.0",
Http.Method.GET,
UriPath.create("/"),
Expand Down

0 comments on commit c8fea30

Please sign in to comment.