Skip to content

Commit

Permalink
Support absolute-form URI in HTTP requests (#7201)
Browse files Browse the repository at this point in the history
* Support absolute-form URI in HTTP requests
This changeset drops the scheme and authority parts from the URI in two places where we interact with the netty uri.
There is also a workaround for netty/netty#12301 .
Fixes #7193

* work around infinite loop in BufferLeakDetection

* add a url without authority to the test
  • Loading branch information
yawkat committed Apr 11, 2022
1 parent 7e32263 commit f1f49b4
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 9 deletions.
Expand Up @@ -35,6 +35,7 @@
import io.netty.util.DefaultAttributeMap;

import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.util.Collection;
import java.util.Locale;
Expand Down Expand Up @@ -72,8 +73,22 @@ public abstract class AbstractNettyHttpRequest<B> extends DefaultAttributeMap im
public AbstractNettyHttpRequest(io.netty.handler.codec.http.HttpRequest nettyRequest, ConversionService conversionService) {
this.nettyRequest = nettyRequest;
this.conversionService = conversionService;
String fullUri = nettyRequest.uri();
this.uri = URI.create(fullUri);
URI fullUri = URI.create(nettyRequest.uri());
if (fullUri.getAuthority() != null || fullUri.getScheme() != null) {
// http://example.com/foo -> /foo
try {
fullUri = new URI(
null, // scheme
null, // authority
fullUri.getPath(),
fullUri.getQuery(),
fullUri.getFragment()
);
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}
this.uri = fullUri;
this.httpMethodName = nettyRequest.method().name();
this.httpMethod = HttpMethod.parse(httpMethodName);
}
Expand Down Expand Up @@ -153,7 +168,7 @@ public HttpParameters getParameters() {
synchronized (this) { // double check
httpParameters = this.httpParameters;
if (httpParameters == null) {
httpParameters = decodeParameters(nettyRequest.uri());
httpParameters = decodeParameters();
this.httpParameters = httpParameters;
}
}
Expand Down Expand Up @@ -210,7 +225,7 @@ public String getPath() {
synchronized (this) { // double check
path = this.path;
if (path == null) {
path = decodePath(nettyRequest.uri());
path = decodePath();
this.path = path;
}
}
Expand All @@ -228,17 +243,17 @@ public String getPath() {
* @param uri The URI
* @return The query string decoder
*/
protected QueryStringDecoder createDecoder(String uri) {
protected final QueryStringDecoder createDecoder(URI uri) {
Charset charset = getCharacterEncoding();
return charset != null ? new QueryStringDecoder(uri, charset) : new QueryStringDecoder(uri);
}

private String decodePath(String uri) {
private String decodePath() {
QueryStringDecoder queryStringDecoder = createDecoder(uri);
return queryStringDecoder.rawPath();
}

private NettyHttpParameters decodeParameters(String uri) {
private NettyHttpParameters decodeParameters() {
QueryStringDecoder queryStringDecoder = createDecoder(uri);
return new NettyHttpParameters(queryStringDecoder.parameters(), conversionService, null);
}
Expand Down
Expand Up @@ -665,7 +665,7 @@ public MutableHttpParameters getParameters() {
synchronized (this) { // double check
httpParameters = this.httpParameters;
if (httpParameters == null) {
QueryStringDecoder queryStringDecoder = createDecoder(uri.toString());
QueryStringDecoder queryStringDecoder = createDecoder(uri);
httpParameters = new NettyHttpParameters(queryStringDecoder.parameters(), conversionService, null);
this.httpParameters = httpParameters;
}
Expand Down
@@ -0,0 +1,105 @@
package io.micronaut.http.server.netty

import io.micronaut.context.ApplicationContext
import io.micronaut.context.annotation.Requires
import io.micronaut.http.annotation.Controller
import io.micronaut.http.annotation.Get
import io.micronaut.runtime.server.EmbeddedServer
import io.netty.channel.ChannelHandlerContext
import io.netty.channel.ChannelOutboundHandlerAdapter
import io.netty.channel.ChannelPromise
import io.netty.channel.embedded.EmbeddedChannel
import io.netty.handler.codec.http.DefaultHttpRequest
import io.netty.handler.codec.http.FullHttpResponse
import io.netty.handler.codec.http.HttpClientCodec
import io.netty.handler.codec.http.HttpHeaderNames
import io.netty.handler.codec.http.HttpHeaderValues
import io.netty.handler.codec.http.HttpMethod
import io.netty.handler.codec.http.HttpObjectAggregator
import io.netty.handler.codec.http.HttpResponseStatus
import io.netty.handler.codec.http.HttpVersion
import jakarta.inject.Singleton
import spock.lang.Issue
import spock.lang.Specification

import java.nio.charset.StandardCharsets

class RequestLineSpec extends Specification {
@Issue('https://github.com/micronaut-projects/micronaut-core/issues/7193')
def 'test different request lines http 1'() {
given:
ApplicationContext ctx = ApplicationContext.run([
'spec.name': 'RequestLineSpec',
])
def embeddedServer = (NettyHttpServer) ctx.getBean(EmbeddedServer)

def serverEmbeddedChannel = embeddedServer.buildEmbeddedChannel(false)

def clientEmbeddedChannel = new EmbeddedChannel()

serverEmbeddedChannel.pipeline()
.addFirst(new ChannelOutboundHandlerAdapter() {
@Override
void write(ChannelHandlerContext ctx_, Object msg, ChannelPromise promise) throws Exception {
// forward to client
clientEmbeddedChannel.writeOneInbound(msg)
}

@Override
void flush(ChannelHandlerContext ctx_) throws Exception {
clientEmbeddedChannel.flushInbound()
}
})
clientEmbeddedChannel.pipeline()
.addLast(new ChannelOutboundHandlerAdapter() {
@Override
void write(ChannelHandlerContext ctx_, Object msg, ChannelPromise promise) throws Exception {
// forward to server
serverEmbeddedChannel.writeOneInbound(msg)
}

@Override
void flush(ChannelHandlerContext ctx_) throws Exception {
serverEmbeddedChannel.flushInbound()
}
})
.addLast(new HttpClientCodec())
.addLast(new HttpObjectAggregator(1024))

def request1 = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, uri)
request1.headers().add(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE)

when:
clientEmbeddedChannel.writeOneOutbound(request1)
clientEmbeddedChannel.flushOutbound()
serverEmbeddedChannel.runPendingTasks()

then:
FullHttpResponse response = clientEmbeddedChannel.readInbound()
response.status() == HttpResponseStatus.OK
response.content().toString(StandardCharsets.UTF_8) == 'bar'

cleanup:
response.release()
clientEmbeddedChannel.close()
serverEmbeddedChannel.close()
ctx.close()

where:
uri << [
'/foo', // origin-form
'http://example.com/foo', // absolute-form
'http:///foo', // weird form
]
}

@Singleton
@Controller
@Requires(property = 'spec.name', value = 'RequestLineSpec')
public static class SimpleController {
@Get('/foo')
public String foo() {
return "bar"
}
}
}
Expand Up @@ -16,6 +16,7 @@ class BufferLeakDetection<T> extends ResourceLeakDetector<T> {

private static final List<ResourceLeakDetector<?>> allDetectors = new CopyOnWriteArrayList<>()

private static final String BASE_CANARY_STRING = "canary-" + UUID.randomUUID() + "-"
private static volatile String canaryString = null

private static volatile String currentTest = null
Expand Down Expand Up @@ -54,13 +55,21 @@ class BufferLeakDetection<T> extends ResourceLeakDetector<T> {

// adapted from https://github.com/netty/netty/blob/86603872776e3ff5a60dea3c104358e486eed588/common/src/test/java/io/netty/util/ResourceLeakDetectorTest.java
private static synchronized void triggerGc() {
// timeout of last resort for the loop below. use nanoTime because it's monotonic
long startTime = System.nanoTime()

// need to randomize this every time, since ResourceLeakDetector will deduplicate leaks
canaryString = "canary-" + UUID.randomUUID()
canaryString = BASE_CANARY_STRING + UUID.randomUUID()
canaryDetected = false

leakCanary()

do {
if (System.nanoTime() - startTime > 30_000_000_000L) {
LOG.warn("Canary leak detection failed.")
break
}

// Trigger GC.
System.gc();

Expand Down Expand Up @@ -101,6 +110,10 @@ class BufferLeakDetection<T> extends ResourceLeakDetector<T> {
canaryDetected = true
return
}
if (records.contains(BASE_CANARY_STRING)) {
// probably a canary from another run that ran into a timeout, drop
return
}

leakDetected = true
super.reportTracedLeak(resourceType, records)
Expand Down

0 comments on commit f1f49b4

Please sign in to comment.