Skip to content

Commit

Permalink
Fix HTTP/2 304 responses for files don't associate the correct stream…
Browse files Browse the repository at this point in the history
… ID (#6752)

* Fix HTTP 304 responses for files don't associate the correct stream ID

* fix checkstyle / make final

* checkstyle

* - test case
- use common method in NettyStreamedCustomizableResponseType too

Co-authored-by: yawkat <jonas.konrad@oracle.com>
  • Loading branch information
graemerocher and yawkat committed Jan 13, 2022
1 parent 738fe43 commit 9a087a7
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 17 deletions.
Expand Up @@ -57,6 +57,7 @@
import io.netty.handler.codec.http.DefaultLastHttpContent;
import io.netty.handler.codec.http.EmptyHttpHeaders;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpResponse;
import io.netty.handler.codec.http.QueryStringDecoder;
import io.netty.handler.codec.http.cookie.ClientCookieEncoder;
import io.netty.handler.codec.http.multipart.AbstractHttpData;
Expand Down Expand Up @@ -176,6 +177,24 @@ public NettyHttpRequest(io.netty.handler.codec.http.HttpRequest nettyRequest,
this.body = SupplierUtil.memoizedNonEmpty(() -> Optional.ofNullable((T) buildBody()));
}

/**
* Prepares a response based on this HTTP/2 request if HTTP/2 is enabled.
*
* @param finalResponse The response to prepare, never {@code null}
*/
@Internal
public final void prepareHttp2ResponseIfNecessary(@NonNull HttpResponse finalResponse) {
final io.micronaut.http.HttpVersion httpVersion = getHttpVersion();
final boolean isHttp2 = httpVersion == io.micronaut.http.HttpVersion.HTTP_2_0;
if (isHttp2) {
final io.netty.handler.codec.http.HttpHeaders nativeHeaders = nettyRequest.headers();
final String streamId = nativeHeaders.get(STREAM_ID);
if (streamId != null) {
finalResponse.headers().set(STREAM_ID, streamId);
}
}
}

@Override
public MutableHttpRequest<T> mutate() {
return new NettyMutableHttpRequest();
Expand Down
Expand Up @@ -144,8 +144,8 @@
class RoutingInBoundHandler extends SimpleChannelInboundHandler<io.micronaut.http.HttpRequest<?>> {

private static final Logger LOG = LoggerFactory.getLogger(RoutingInBoundHandler.class);
/**
* Also present in {@link RouteExecutor}
/*
* Also present in {@link RouteExecutor}.
*/
private static final Pattern IGNORABLE_ERROR_MESSAGE = Pattern.compile(
"^.*(?:connection.*(?:reset|closed|abort|broken)|broken.*pipe).*$", Pattern.CASE_INSENSITIVE);
Expand Down
Expand Up @@ -22,6 +22,7 @@
import io.micronaut.http.MutableHttpHeaders;
import io.micronaut.http.MutableHttpResponse;
import io.micronaut.http.netty.NettyMutableHttpResponse;
import io.micronaut.http.server.netty.NettyHttpRequest;
import io.micronaut.http.server.netty.configuration.NettyHttpServerConfiguration;
import io.micronaut.http.server.netty.types.NettyCustomizableResponseTypeHandler;
import io.micronaut.http.server.netty.types.NettyFileCustomizableResponseType;
Expand Down Expand Up @@ -87,6 +88,9 @@ public void handle(Object obj, HttpRequest<?> request, MutableHttpResponse<?> re
long fileLastModifiedSeconds = lastModified / 1000;
if (ifModifiedSinceDateSeconds == fileLastModifiedSeconds) {
FullHttpResponse nettyResponse = notModified(response);
if (request instanceof NettyHttpRequest) {
((NettyHttpRequest<?>) request).prepareHttp2ResponseIfNecessary(nettyResponse);
}
context.writeAndFlush(nettyResponse);
return;
}
Expand Down
Expand Up @@ -19,7 +19,6 @@
import io.micronaut.http.HttpRequest;
import io.micronaut.http.MediaType;
import io.micronaut.http.MutableHttpResponse;
import io.micronaut.http.netty.AbstractNettyHttpRequest;
import io.micronaut.http.netty.NettyMutableHttpResponse;
import io.micronaut.http.server.netty.NettyHttpRequest;
import io.micronaut.http.server.netty.SmartHttpContentCompressor;
Expand Down Expand Up @@ -119,14 +118,8 @@ public void write(HttpRequest<?> request, MutableHttpResponse<?> response, Chann

// Write the request data
final DefaultHttpResponse finalResponse = new DefaultHttpResponse(nettyResponse.getNettyHttpVersion(), nettyResponse.getNettyHttpStatus(), nettyResponse.getNettyHeaders());
final io.micronaut.http.HttpVersion httpVersion = request.getHttpVersion();
final boolean isHttp2 = httpVersion == io.micronaut.http.HttpVersion.HTTP_2_0;
if (isHttp2 && request instanceof NettyHttpRequest) {
final io.netty.handler.codec.http.HttpHeaders nativeHeaders = ((NettyHttpRequest<?>) request).getNativeRequest().headers();
final String streamId = nativeHeaders.get(AbstractNettyHttpRequest.STREAM_ID);
if (streamId != null) {
finalResponse.headers().set(AbstractNettyHttpRequest.STREAM_ID, streamId);
}
if (request instanceof NettyHttpRequest) {
((NettyHttpRequest<?>) request).prepareHttp2ResponseIfNecessary(finalResponse);
}
context.write(finalResponse, context.voidPromise());

Expand Down Expand Up @@ -159,4 +152,5 @@ public void write(HttpRequest<?> request, MutableHttpResponse<?> response, Chann
throw new IllegalArgumentException("Unsupported response type. Not a Netty response: " + response);
}
}

}
Expand Up @@ -58,12 +58,8 @@ default void write(HttpRequest<?> request, MutableHttpResponse<?> response, Chan
final DefaultHttpResponse finalResponse = new DefaultHttpResponse(nettyResponse.getNettyHttpVersion(), nettyResponse.getNettyHttpStatus(), nettyResponse.getNettyHeaders());
final io.micronaut.http.HttpVersion httpVersion = request.getHttpVersion();
final boolean isHttp2 = httpVersion == io.micronaut.http.HttpVersion.HTTP_2_0;
if (isHttp2 && request instanceof NettyHttpRequest) {
final io.netty.handler.codec.http.HttpHeaders nativeHeaders = ((NettyHttpRequest<?>) request).getNativeRequest().headers();
final String streamId = nativeHeaders.get(AbstractNettyHttpRequest.STREAM_ID);
if (streamId != null) {
finalResponse.headers().set(AbstractNettyHttpRequest.STREAM_ID, streamId);
}
if (request instanceof NettyHttpRequest) {
((NettyHttpRequest<?>) request).prepareHttp2ResponseIfNecessary(finalResponse);
}
InputStream inputStream = getInputStream();
// can be null if the stream was closed
Expand Down
@@ -0,0 +1,125 @@
package io.micronaut.http.server.netty.resources

import io.micronaut.context.ApplicationContext
import io.micronaut.http.HttpHeaders
import io.micronaut.http.netty.AbstractNettyHttpRequest
import io.micronaut.runtime.server.EmbeddedServer
import io.netty.bootstrap.Bootstrap
import io.netty.channel.ChannelHandlerContext
import io.netty.channel.ChannelInitializer
import io.netty.channel.ChannelOption
import io.netty.channel.SimpleChannelInboundHandler
import io.netty.channel.nio.NioEventLoopGroup
import io.netty.channel.socket.SocketChannel
import io.netty.channel.socket.nio.NioSocketChannel
import io.netty.handler.codec.http.DefaultFullHttpRequest
import io.netty.handler.codec.http.HttpMethod
import io.netty.handler.codec.http.HttpResponse
import io.netty.handler.codec.http.HttpResponseStatus
import io.netty.handler.codec.http.HttpVersion
import io.netty.handler.codec.http2.DefaultHttp2Connection
import io.netty.handler.codec.http2.Http2SecurityUtil
import io.netty.handler.codec.http2.Http2Settings
import io.netty.handler.codec.http2.HttpConversionUtil
import io.netty.handler.codec.http2.HttpToHttp2ConnectionHandlerBuilder
import io.netty.handler.codec.http2.InboundHttp2ToHttpAdapterBuilder
import io.netty.handler.ssl.ApplicationProtocolConfig
import io.netty.handler.ssl.ApplicationProtocolNames
import io.netty.handler.ssl.ApplicationProtocolNegotiationHandler
import io.netty.handler.ssl.SslContextBuilder
import io.netty.handler.ssl.SupportedCipherSuiteFilter
import io.netty.handler.ssl.util.InsecureTrustManagerFactory
import org.jetbrains.annotations.NotNull
import spock.lang.Specification

import java.time.Instant
import java.time.ZoneOffset
import java.time.format.DateTimeFormatter
import java.util.concurrent.CompletableFuture

class Http2StaticResourceCacheSpec extends Specification {
def 'response should have proper stream id'() {
given:
def tempFile = File.createTempFile("Http2StaticResourceCacheSpec", ".html")
tempFile.write("<html><head></head><body>HTML Page from static file</body></html>")
def tempSubDir = new File(tempFile.getParentFile(), "doesntexist")
def app = ApplicationContext.run([
'micronaut.server.http-version': '2.0',
'micronaut.ssl.enabled': true,
'micronaut.ssl.port': -1,
'micronaut.ssl.buildSelfSigned': true,
'micronaut.router.static-resources.default.paths': ['classpath:public', 'file:' + tempFile.parent, 'file:' + tempSubDir.absolutePath]
])
def embeddedServer = app.getBean(EmbeddedServer)
embeddedServer.start()

def sslContext = SslContextBuilder.forClient()
.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE)
.trustManager(InsecureTrustManagerFactory.INSTANCE)
.applicationProtocolConfig(new ApplicationProtocolConfig(
ApplicationProtocolConfig.Protocol.ALPN,
// NO_ADVERTISE is currently the only mode supported by both OpenSsl and JDK providers.
ApplicationProtocolConfig.SelectorFailureBehavior.NO_ADVERTISE,
// ACCEPT is currently the only mode supported by both OpenSsl and JDK providers.
ApplicationProtocolConfig.SelectedListenerFailureBehavior.ACCEPT,
ApplicationProtocolNames.HTTP_2))
.build()
def completion = new CompletableFuture<HttpResponse>()
def bootstrap = new Bootstrap()
.remoteAddress(embeddedServer.host, embeddedServer.port)
.group(new NioEventLoopGroup())
.channel(NioSocketChannel.class)
.option(ChannelOption.AUTO_READ, true)
.handler(new ChannelInitializer<SocketChannel>() {
@Override
protected void initChannel(@NotNull SocketChannel ch) throws Exception {
def connection = new DefaultHttp2Connection(false)
def connectionHandler = new HttpToHttp2ConnectionHandlerBuilder()
.initialSettings(Http2Settings.defaultSettings())
.frameListener(new InboundHttp2ToHttpAdapterBuilder(connection)
.maxContentLength(Integer.MAX_VALUE)
.propagateSettings(false)
.build())
.connection(connection)
.build()
ch.pipeline()
.addLast(sslContext.newHandler(ch.alloc(), embeddedServer.host, embeddedServer.port))
.addLast(new ApplicationProtocolNegotiationHandler('') {
@Override
protected void configurePipeline(ChannelHandlerContext ctx, String protocol) throws Exception {
if (ApplicationProtocolNames.HTTP_2 != protocol) {
throw new AssertionError((Object) protocol)
}
ctx.pipeline()
.addLast(connectionHandler)
.addLast(new SimpleChannelInboundHandler<HttpResponse>() {
@Override
protected void channelRead0(ChannelHandlerContext ctx_, HttpResponse msg) throws Exception {
completion.complete(msg)
}
})


def request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, '/' + tempFile.getName())
request.headers().add(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), 'https')
request.headers().add(AbstractNettyHttpRequest.STREAM_ID, 3)
request.headers().add(HttpHeaders.IF_MODIFIED_SINCE, Instant.ofEpochMilli(tempFile.lastModified()).atZone(ZoneOffset.UTC).format(DateTimeFormatter.RFC_1123_DATE_TIME))
ctx.channel().writeAndFlush(request)
}
})
}
})

when:
def channel = bootstrap.connect().await().channel()
def response = completion.get()
then:
response.status() == HttpResponseStatus.NOT_MODIFIED
response.headers().get(AbstractNettyHttpRequest.STREAM_ID) == '3'

cleanup:
tempFile.delete()
channel.close()
embeddedServer.close()
}
}

0 comments on commit 9a087a7

Please sign in to comment.