Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support absolute-form URI in HTTP requests #7201

Merged
merged 3 commits into from Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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