From 18b085f1bb05fc1d3e712e3362ff1da154de74a4 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Wed, 29 Nov 2023 14:10:03 +0100 Subject: [PATCH 1/4] Allow to skip parsing the request's url into `URI` --- .../client/netty/NettyClientHttpRequest.java | 21 +-- .../http/netty/AbstractNettyHttpRequest.java | 128 ++++++++++-------- .../http/netty/NettyHttpHeaders.java | 119 ++++++++++++++-- .../netty/HttpToHttpsRedirectHandler.java | 2 +- .../http/server/netty/NettyHttpRequest.java | 64 +-------- .../server/netty/NettyRequestLifecycle.java | 2 - .../MalformedUriDisabledValidationSpec.groovy | 86 ++++++++++++ .../http/server/HttpServerConfiguration.java | 19 +++ .../http/server/RequestLifecycle.java | 107 +++++++++------ .../micronaut/http/server/RouteExecutor.java | 20 +-- .../java/io/micronaut/http/HttpHeaders.java | 26 +++- .../java/io/micronaut/http/HttpRequest.java | 2 +- .../http/simple/SimpleHttpRequest.java | 8 +- .../java/io/micronaut/http/util/HttpUtil.java | 2 +- 14 files changed, 396 insertions(+), 210 deletions(-) create mode 100644 http-server-netty/src/test/groovy/io/micronaut/http/server/netty/errors/MalformedUriDisabledValidationSpec.groovy diff --git a/http-client/src/main/java/io/micronaut/http/client/netty/NettyClientHttpRequest.java b/http-client/src/main/java/io/micronaut/http/client/netty/NettyClientHttpRequest.java index 50777dee20a..8626f9df508 100644 --- a/http-client/src/main/java/io/micronaut/http/client/netty/NettyClientHttpRequest.java +++ b/http-client/src/main/java/io/micronaut/http/client/netty/NettyClientHttpRequest.java @@ -79,19 +79,6 @@ public class NettyClientHttpRequest implements MutableHttpRequest, NettyHt private NettyHttpParameters httpParameters; private ConversionService conversionService = ConversionService.SHARED; - /** - * This constructor is actually required for the case of non-standard http methods. - * - * @param httpMethod The http method. CUSTOM value is used for non-standard - * @param uri The uri - * @param httpMethodName Method name. Is the same as httpMethod.name() value for standard http methods. - */ - NettyClientHttpRequest(HttpMethod httpMethod, URI uri, String httpMethodName) { - this.httpMethod = httpMethod; - this.uri = uri; - this.httpMethodName = httpMethodName; - } - /** * @param httpMethod The Http method * @param uri The URI @@ -104,11 +91,13 @@ public class NettyClientHttpRequest implements MutableHttpRequest, NettyHt * This constructor is actually required for the case of non-standard http methods. * * @param httpMethod The http method. CUSTOM value is used for non-standard - * @param uri The uri + * @param url The uri * @param httpMethodName Method name. Is the same as httpMethod.name() value for standard http methods. */ - NettyClientHttpRequest(HttpMethod httpMethod, String uri, String httpMethodName) { - this(httpMethod, URI.create(uri), httpMethodName); + NettyClientHttpRequest(HttpMethod httpMethod, String url, String httpMethodName) { + this.httpMethod = httpMethod; + this.uri = URI.create(url); + this.httpMethodName = httpMethodName; } @Override diff --git a/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java b/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java index 3651f4058ad..2f5536b9349 100644 --- a/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java +++ b/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java @@ -22,7 +22,6 @@ import io.micronaut.http.HttpMethod; import io.micronaut.http.HttpParameters; import io.micronaut.http.HttpRequest; -import io.micronaut.http.MediaType; import io.micronaut.http.netty.stream.DefaultStreamedHttpRequest; import io.micronaut.http.netty.stream.StreamedHttpRequest; import io.netty.handler.codec.http.DefaultFullHttpRequest; @@ -34,9 +33,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.Charset; -import java.util.Collection; -import java.util.Locale; -import java.util.Optional; /** * Abstract implementation of {@link HttpRequest} for Netty. @@ -51,15 +47,13 @@ public abstract class AbstractNettyHttpRequest extends DefaultAttributeMap im protected final io.netty.handler.codec.http.HttpRequest nettyRequest; protected final ConversionService conversionService; protected final HttpMethod httpMethod; - protected final URI uri; + protected final String url; protected final String httpMethodName; + private URI uri; private NettyHttpParameters httpParameters; - private Optional mediaType; private Charset charset; - private Optional locale; private String path; - private Collection accept; /** * @param nettyRequest The Http netty request @@ -68,22 +62,7 @@ public abstract class AbstractNettyHttpRequest extends DefaultAttributeMap im public AbstractNettyHttpRequest(io.netty.handler.codec.http.HttpRequest nettyRequest, ConversionService conversionService) { this.nettyRequest = nettyRequest; this.conversionService = conversionService; - URI fullUri = URI.create(nettyRequest.uri()); - if (fullUri.getAuthority() != null || fullUri.getScheme() != null) { - // https://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.url = nettyRequest.uri(); this.httpMethodName = nettyRequest.method().name(); this.httpMethod = HttpMethod.parse(httpMethodName); } @@ -163,23 +142,6 @@ public HttpParameters getParameters() { return params; } - @Override - public Collection accept() { - if (accept == null) { - accept = HttpRequest.super.accept(); - } - return accept; - } - - @Override - @SuppressWarnings("java:S2789") // performance opt - public Optional getContentType() { - if (mediaType == null) { - mediaType = HttpRequest.super.getContentType(); - } - return mediaType; - } - @Override public Charset getCharacterEncoding() { if (charset == null) { @@ -188,15 +150,6 @@ public Charset getCharacterEncoding() { return charset; } - @Override - @SuppressWarnings("java:S2789") // performance opt - public Optional getLocale() { - if (locale == null) { - locale = HttpRequest.super.getLocale(); - } - return locale; - } - @Override public HttpMethod getMethod() { return httpMethod; @@ -204,7 +157,17 @@ public HttpMethod getMethod() { @Override public URI getUri() { - return this.uri; + URI u = this.uri; + if (u == null) { + synchronized (this) { // double check + u = this.uri; + if (u == null) { + u = createURI(url); + this.uri = u; + } + } + } + return u; } @Override @@ -214,7 +177,7 @@ public String getPath() { synchronized (this) { // double check p = this.path; if (p == null) { - p = decodePath(); + p = parsePath(url); this.path = p; } } @@ -223,7 +186,7 @@ public String getPath() { } /** - * @param characterEncoding The charactger encoding + * @param characterEncoding The character encoding * @return The Charset */ protected abstract Charset initCharset(Charset characterEncoding); @@ -238,13 +201,8 @@ protected final QueryStringDecoder createDecoder(URI uri) { return cs != null ? new QueryStringDecoder(uri, cs) : new QueryStringDecoder(uri); } - private String decodePath() { - QueryStringDecoder queryStringDecoder = createDecoder(uri); - return queryStringDecoder.rawPath(); - } - private NettyHttpParameters decodeParameters() { - QueryStringDecoder queryStringDecoder = createDecoder(uri); + QueryStringDecoder queryStringDecoder = createDecoder(getUri()); return new NettyHttpParameters(queryStringDecoder.parameters(), conversionService, null); } @@ -252,4 +210,56 @@ private NettyHttpParameters decodeParameters() { public String getMethodName() { return httpMethodName; } + + private static URI createURI(String url) { + URI fullUri = URI.create(url); + if (fullUri.getAuthority() != null || fullUri.getScheme() != null) { + // https://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); + } + } + return fullUri; + } + + /** + * Extract the path out of the uri. + * https://github.com/eclipse-vertx/vert.x/blob/master/src/main/java/io/vertx/core/http/impl/HttpUtils.java + */ + private static String parsePath(String uri) { + if (uri.isEmpty()) { + return ""; + } + int i; + if (uri.charAt(0) == '/') { + i = 0; + } else { + i = uri.indexOf("://"); + if (i == -1) { + i = 0; + } else { + i = uri.indexOf('/', i + 3); + if (i == -1) { + // contains no / + return "/"; + } + } + } + int queryStart = uri.indexOf('?', i); + if (queryStart == -1) { + queryStart = uri.length(); + if (i == 0) { + return uri; + } + } + return uri.substring(i, queryStart); + } } diff --git a/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java b/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java index 22101cdb2fd..06908e9c449 100644 --- a/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java +++ b/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java @@ -16,17 +16,18 @@ package io.micronaut.http.netty; import io.micronaut.core.annotation.Internal; +import io.micronaut.core.annotation.Nullable; import io.micronaut.core.convert.ArgumentConversionContext; import io.micronaut.core.convert.ConversionService; import io.micronaut.core.type.MutableHeaders; import io.micronaut.http.HttpHeaderValues; +import io.micronaut.http.HttpHeaders; import io.micronaut.http.MediaType; import io.micronaut.http.MutableHttpHeaders; import io.micronaut.http.util.HttpHeadersUtil; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaderValidationUtil; -import jakarta.annotation.Nullable; import java.net.URI; import java.nio.charset.Charset; @@ -53,11 +54,25 @@ * @since 1.0 */ @Internal +@SuppressWarnings("OptionalUsedAsFieldOrParameterType") public class NettyHttpHeaders implements MutableHttpHeaders { private final io.netty.handler.codec.http.HttpHeaders nettyHeaders; private ConversionService conversionService; + @Nullable + private OptionalLong contentLength; + @Nullable + private Optional contentType; + @Nullable + private Optional origin; + @Nullable + private List accept; + @Nullable + private Optional acceptCharset; + @Nullable + private Optional acceptLanguage; + /** * @param nettyHeaders The Netty Http headers * @param conversionService The conversion service @@ -141,6 +156,7 @@ public Optional findFirst(CharSequence name) { public MutableHttpHeaders add(CharSequence header, CharSequence value) { validateHeader(header, value); nettyHeaders.add(header, value); + onModify(header); return this; } @@ -148,9 +164,26 @@ public MutableHttpHeaders add(CharSequence header, CharSequence value) { public MutableHeaders set(CharSequence header, CharSequence value) { validateHeader(header, value); nettyHeaders.set(header, value); + onModify(header); return this; } + private void onModify(CharSequence header) { + if (contentType != null && header.equals(HttpHeaders.CONTENT_TYPE)) { + contentType = null; + } else if (contentLength != null && header.equals(HttpHeaders.CONTENT_LENGTH)) { + contentLength = null; + } else if (accept != null && header.equals(HttpHeaders.ACCEPT)) { + accept = null; + } else if (acceptCharset != null && header.equals(HttpHeaders.ACCEPT_CHARSET)) { + acceptCharset = null; + } else if (acceptLanguage != null && header.equals(HttpHeaders.ACCEPT_LANGUAGE)) { + acceptLanguage = null; + } else if (origin != null && header.equals(HttpHeaders.ORIGIN)) { + origin = null; + } + } + /** * Like {@link #set(CharSequence, CharSequence)} but without header validation. * @@ -159,6 +192,7 @@ public MutableHeaders set(CharSequence header, CharSequence value) { */ public void setUnsafe(CharSequence header, CharSequence value) { nettyHeaders.set(header, value); + onModify(header); } public static void validateHeader(CharSequence name, CharSequence value) { @@ -173,6 +207,7 @@ public static void validateHeader(CharSequence name, CharSequence value) { @Override public MutableHttpHeaders remove(CharSequence header) { nettyHeaders.remove(header); + onModify(header); return this; } @@ -271,6 +306,15 @@ public void setConversionService(ConversionService conversionService) { @Override public Optional contentType() { + Optional cachedContentType = contentType; + if (cachedContentType == null) { + cachedContentType = resolveContentType(); + contentType = cachedContentType; + } + return cachedContentType; + } + + private Optional resolveContentType() { // optimization to avoid ConversionService String str = get(HttpHeaderNames.CONTENT_TYPE); if (str != null) { @@ -284,11 +328,20 @@ public Optional contentType() { @Override public OptionalLong contentLength() { + OptionalLong cachedContentLength = contentLength; + if (cachedContentLength == null) { + cachedContentLength = resolveContentLength(); + contentLength = cachedContentLength; + } + return cachedContentLength; + } + + private OptionalLong resolveContentLength() { // optimization to avoid ConversionService - Optional str = findFirst(HttpHeaderNames.CONTENT_LENGTH); - if (str.isPresent()) { + String str = get(HttpHeaderNames.CONTENT_LENGTH); + if (str != null) { try { - return OptionalLong.of(Long.parseLong(str.get())); + return OptionalLong.of(Long.parseLong(str)); } catch (NumberFormatException ignored) { } } @@ -297,41 +350,79 @@ public OptionalLong contentLength() { @Override public List accept() { + List cachedAccept = accept; + if (cachedAccept == null) { + cachedAccept = resolveAccept(); + accept = cachedAccept; + } + return cachedAccept; + } + + private List resolveAccept() { // use HttpHeaderNames instead of HttpHeaders return MediaType.orderedOf(getAll(HttpHeaderNames.ACCEPT)); } - @Nullable @Override - public Charset acceptCharset() { + public Optional findAcceptCharset() { + Optional cachedAcceptCharset = acceptCharset; + if (cachedAcceptCharset == null) { + cachedAcceptCharset = resolveAcceptCharset(); + acceptCharset = cachedAcceptCharset; + } + return cachedAcceptCharset; + } + + private Optional resolveAcceptCharset() { String text = get(HttpHeaderNames.ACCEPT_CHARSET); if (text == null) { - return null; + return Optional.empty(); } text = HttpHeadersUtil.splitAcceptHeader(text); if (text != null) { try { - return Charset.forName(text); + return Optional.of(Charset.forName(text)); } catch (Exception ignored) { } } // default to UTF-8 - return StandardCharsets.UTF_8; + return Optional.of(StandardCharsets.UTF_8); } - @Nullable @Override - public Locale acceptLanguage() { + public Optional findAcceptLanguage() { + Optional cachedAcceptLanguage = acceptLanguage; + if (cachedAcceptLanguage == null) { + cachedAcceptLanguage = resolveAcceptLanguage(); + acceptLanguage = cachedAcceptLanguage; + } + return cachedAcceptLanguage; + } + + private Optional resolveAcceptLanguage() { String text = get(HttpHeaderNames.ACCEPT_LANGUAGE); if (text == null) { - return null; + return Optional.empty(); } String part = HttpHeadersUtil.splitAcceptHeader(text); - return part == null ? Locale.getDefault() : Locale.forLanguageTag(part); + return Optional.ofNullable(part == null ? Locale.getDefault() : Locale.forLanguageTag(part)); } @Override public Optional getOrigin() { - return findFirst(HttpHeaderNames.ORIGIN); + Optional cachedOrigin = origin; + if (cachedOrigin == null) { + cachedOrigin = resolveOrigin(); + origin = cachedOrigin; + } + return cachedOrigin; + } + + private Optional resolveOrigin() { + Optional cachedOrigin = origin; + if (cachedOrigin == null) { + cachedOrigin = findFirst(HttpHeaderNames.ORIGIN); + } + return cachedOrigin; } } diff --git a/http-server-netty/src/main/java/io/micronaut/http/server/netty/HttpToHttpsRedirectHandler.java b/http-server-netty/src/main/java/io/micronaut/http/server/netty/HttpToHttpsRedirectHandler.java index 4b0783bdd55..bce2bb52d9a 100644 --- a/http-server-netty/src/main/java/io/micronaut/http/server/netty/HttpToHttpsRedirectHandler.java +++ b/http-server-netty/src/main/java/io/micronaut/http/server/netty/HttpToHttpsRedirectHandler.java @@ -51,7 +51,7 @@ record HttpToHttpsRedirectHandler( @Override public void accept(ChannelHandlerContext ctx, io.netty.handler.codec.http.HttpRequest request, PipeliningServerHandler.OutboundAccess outboundAccess) { - NettyHttpRequest strippedRequest = NettyHttpRequest.createSafe(request, ctx, conversionService, serverConfiguration); + NettyHttpRequest strippedRequest = new NettyHttpRequest<>(request, ctx, conversionService, serverConfiguration); UriBuilder uriBuilder = UriBuilder.of(hostResolver.resolve(strippedRequest)); strippedRequest.release(); diff --git a/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyHttpRequest.java b/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyHttpRequest.java index b1fd2d36784..d702632ebb1 100644 --- a/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyHttpRequest.java +++ b/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyHttpRequest.java @@ -56,7 +56,6 @@ import io.micronaut.http.server.netty.body.ImmediateByteBody; import io.micronaut.http.server.netty.body.ImmediateMultiObjectBody; import io.micronaut.http.server.netty.body.ImmediateSingleObjectBody; -import io.micronaut.http.server.netty.configuration.NettyHttpServerConfiguration; import io.micronaut.http.server.netty.multipart.NettyCompletedFileUpload; import io.micronaut.web.router.RouteMatch; import io.netty.buffer.ByteBuf; @@ -163,17 +162,6 @@ public class NettyHttpRequest extends AbstractNettyHttpRequest implements private FormRouteCompleter formRouteCompleter; private ExecutionFlow routeWaitsFor = ExecutionFlow.just(null); - /** - * Set to {@code true} when the {@link #headers} may have been mutated. If this is not the case, - * we can cache some values. - */ - private boolean headersMutated = false; - private final long contentLength; - @Nullable - private final MediaType contentType; - @Nullable - private final String origin; - private final BodyConvertor bodyConvertor = newBodyConvertor(); /** @@ -196,34 +184,6 @@ public NettyHttpRequest(io.netty.handler.codec.http.HttpRequest nettyRequest, this.channelHandlerContext = ctx; this.headers = new NettyHttpHeaders(nettyRequest.headers(), conversionService); this.body = ByteBody.of(nettyRequest); - this.contentLength = headers.contentLength().orElse(-1); - this.contentType = headers.contentType().orElse(null); - this.origin = headers.getOrigin().orElse(null); - } - - public static NettyHttpRequest createSafe(io.netty.handler.codec.http.HttpRequest request, ChannelHandlerContext ctx, ConversionService conversionService, NettyHttpServerConfiguration serverConfiguration) { - try { - return new NettyHttpRequest<>( - request, - ctx, - conversionService, - serverConfiguration - ); - } catch (IllegalArgumentException iae) { - // invalid URI - if (request instanceof StreamedHttpRequest streamed) { - streamed.closeIfNoSubscriber(); - } else { - ((FullHttpRequest) request).release(); - } - - return new NettyHttpRequest<>( - new DefaultFullHttpRequest(request.protocolVersion(), request.method(), "/", Unpooled.EMPTY_BUFFER), - ctx, - conversionService, - serverConfiguration - ); - } } /** @@ -384,11 +344,7 @@ public boolean isSecure() { @Override public Optional getOrigin() { - if (headersMutated) { - return getHeaders().getOrigin(); - } else { - return Optional.ofNullable(origin); - } + return headers.getOrigin(); } @Override @@ -659,12 +615,7 @@ public io.netty.handler.codec.http.HttpRequest toHttpRequestWithoutBody() { @Override public Optional getContentType() { - // this is better than the caching we can do in AbstractNettyHttpRequest - if (headersMutated) { - return headers.contentType(); - } else { - return Optional.ofNullable(contentType); - } + return headers.contentType(); } private BodyConvertor newBodyConvertor() { @@ -686,11 +637,7 @@ public Optional convert(ArgumentConversionContext conversionContext, Object valu @Override public long getContentLength() { - if (headersMutated) { - return super.getContentLength(); - } else { - return contentLength; - } + return headers.contentLength().orElse(-1); } @Override @@ -721,7 +668,7 @@ private static ByteBuffer toByteBuffer(ImmediateByteBody immediateByteB */ private final class NettyMutableHttpRequest implements MutableHttpRequest, NettyHttpRequestBuilder { - private URI uri = NettyHttpRequest.this.uri; + private URI uri; @Nullable private MutableHttpParameters httpParameters; @Nullable @@ -761,7 +708,6 @@ public MutableHttpRequest body(T1 body) { @Override public MutableHttpHeaders getHeaders() { - headersMutated = true; return headers; } @@ -793,7 +739,7 @@ public MutableHttpParameters getParameters() { synchronized (this) { // double check httpParameters = this.httpParameters; if (httpParameters == null) { - QueryStringDecoder queryStringDecoder = createDecoder(uri); + QueryStringDecoder queryStringDecoder = createDecoder(getUri()); httpParameters = new NettyHttpParameters(queryStringDecoder.parameters(), conversionService, null); this.httpParameters = httpParameters; } diff --git a/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyRequestLifecycle.java b/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyRequestLifecycle.java index 151cd5a45a7..3979900623e 100644 --- a/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyRequestLifecycle.java +++ b/http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyRequestLifecycle.java @@ -59,8 +59,6 @@ final class NettyRequestLifecycle extends RequestLifecycle { super(rib.routeExecutor); this.rib = rib; this.outboundAccess = outboundAccess; - - multipartEnabled(rib.multipartEnabled); } void handleNormal(NettyHttpRequest request) { diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/errors/MalformedUriDisabledValidationSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/errors/MalformedUriDisabledValidationSpec.groovy new file mode 100644 index 00000000000..fa65d8307cb --- /dev/null +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/errors/MalformedUriDisabledValidationSpec.groovy @@ -0,0 +1,86 @@ +package io.micronaut.http.server.netty.errors + +import io.micronaut.context.ApplicationContext +import io.micronaut.context.annotation.Requires +import io.micronaut.http.HttpRequest +import io.micronaut.http.HttpResponse +import io.micronaut.http.MediaType +import io.micronaut.http.MutableHttpResponse +import io.micronaut.http.annotation.Controller +import io.micronaut.http.annotation.Error +import io.micronaut.http.annotation.Filter +import io.micronaut.http.annotation.Get +import io.micronaut.http.client.HttpClient +import io.micronaut.http.filter.HttpServerFilter +import io.micronaut.http.filter.ServerFilterChain +import io.micronaut.runtime.server.EmbeddedServer +import jakarta.inject.Singleton +import org.reactivestreams.Publisher +import spock.lang.AutoCleanup +import spock.lang.Retry +import spock.lang.Shared +import spock.lang.Specification + +@Retry +// Retry added because we need to use java.net.URL to test not the Micronaut HTTP client and URL.text from Groovy is unreliable +// sometimes failing for seemingly unknown reasons +class MalformedUriDisabledValidationSpec extends Specification { + + @Shared @AutoCleanup EmbeddedServer embeddedServer = ApplicationContext.run(EmbeddedServer, [ + 'spec.name': 'MalformedUriDisabledValidationSpec', + 'micronaut.server.validate-url': 'false' + ]) + @Shared @AutoCleanup HttpClient client = embeddedServer.applicationContext.createBean(HttpClient, embeddedServer.getURL()) + + void "test malformed URI exceptions"() { + when: + def result = new URL("$embeddedServer.URL/malformed/[]").text + + then: + result == '[]' + } + + void "test filters are called in case of error"() { + given: + OncePerFilter filter = embeddedServer.applicationContext.getBean(OncePerFilter) + + expect: + filter.filterCalled + + when: + def result = new URL("$embeddedServer.URL/malformed/[]").text + + then: + filter.filterCalled + result == '[]' + } + + @Requires(property = "spec.name", value = "MalformedUriDisabledValidationSpec") + @Controller('/malformed') + static class SomeController { + @Get(uri="/{some}", produces = MediaType.TEXT_PLAIN) + String some(String some) throws Exception{ + return some + } + + @Error(exception = URISyntaxException.class, global = true) + HttpResponse exception(HttpRequest request, URISyntaxException e) { + return HttpResponse.ok() + .body("Exception: " + e.getMessage()) + } + } + + @Requires(property = "spec.name", value = "MalformedUriDisabledValidationSpec") + @Singleton + @Filter("/**") + static class OncePerFilter implements HttpServerFilter { + + boolean filterCalled = false + + @Override + Publisher> doFilter(HttpRequest request, ServerFilterChain chain) { + filterCalled = true + return chain.proceed(request) + } + } +} diff --git a/http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java b/http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java index 1035814e5dd..ce4930aa0f6 100644 --- a/http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java +++ b/http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java @@ -147,6 +147,7 @@ public class HttpServerConfiguration implements ServerContextPathProvider { private final ApplicationConfiguration applicationConfiguration; private Charset defaultCharset; private ThreadSelection threadSelection = ThreadSelection.MANUAL; + private boolean validateUrl = true; /** * Default constructor. @@ -533,6 +534,24 @@ public void setDispatchOptionsRequests(boolean dispatchOptionsRequests) { this.dispatchOptionsRequests = dispatchOptionsRequests; } + /** + * If the url should be validated by converting it to {@link java.net.URI}. + * + * @param validateUrl The validate URL value + * @since 4.3.0 + */ + public void setValidateUrl(boolean validateUrl) { + this.validateUrl = validateUrl; + } + + /** + * @return True if the url should be validated + * @since 4.3.0 + */ + public boolean isValidateUrl() { + return validateUrl; + } + /** * Configuration for multipart handling. */ diff --git a/http-server/src/main/java/io/micronaut/http/server/RequestLifecycle.java b/http-server/src/main/java/io/micronaut/http/server/RequestLifecycle.java index 927c941d746..66cdee06e50 100644 --- a/http-server/src/main/java/io/micronaut/http/server/RequestLifecycle.java +++ b/http-server/src/main/java/io/micronaut/http/server/RequestLifecycle.java @@ -71,17 +71,17 @@ public class RequestLifecycle { private static final Logger LOG = LoggerFactory.getLogger(RequestLifecycle.class); private final RouteExecutor routeExecutor; - private boolean multipartEnabled = true; + private final boolean multipartEnabled; + private final boolean validateUrl; /** * @param routeExecutor The route executor to use for route resolution */ protected RequestLifecycle(RouteExecutor routeExecutor) { this.routeExecutor = Objects.requireNonNull(routeExecutor, "routeExecutor"); - } - - protected final void multipartEnabled(boolean multipartEnabled) { - this.multipartEnabled = multipartEnabled; + this.validateUrl = routeExecutor.serverConfiguration.isValidateUrl(); + Optional isMultiPartEnabled = routeExecutor.serverConfiguration.getMultipart().getEnabled(); + this.multipartEnabled = isMultiPartEnabled.isEmpty() || isMultiPartEnabled.get(); } /** @@ -91,48 +91,68 @@ protected final void multipartEnabled(boolean multipartEnabled) { * @return The response to the request. */ protected final ExecutionFlow> normalFlow(HttpRequest request) { - Objects.requireNonNull(request, "request"); - if (!multipartEnabled) { - MediaType contentType = request.getContentType().orElse(null); - if (contentType != null && - contentType.equals(MediaType.MULTIPART_FORM_DATA_TYPE)) { - if (LOG.isDebugEnabled()) { - LOG.debug("Multipart uploads have been disabled via configuration. Rejected request for URI {}, method {}, and content type {}", request.getUri(), - request.getMethodName(), contentType); + try { + Objects.requireNonNull(request, "request"); + if (!multipartEnabled) { + MediaType contentType = request.getContentType().orElse(null); + if (contentType != null && + contentType.equals(MediaType.MULTIPART_FORM_DATA_TYPE)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Multipart uploads have been disabled via configuration. Rejected request for URI {}, method {}, and content type {}", request.getUri(), + request.getMethodName(), contentType); + } + return onStatusError( + request, + HttpResponse.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE), + "Content Type [" + contentType + "] not allowed" + ); } - return onStatusError( - request, - HttpResponse.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE), - "Content Type [" + contentType + "] not allowed" - ); } - } - UriRouteMatch routeMatch = routeExecutor.findRouteMatch(request); - if (routeMatch == null) { - //Check if there is a file for the route before returning route not found - FileCustomizableResponseType fileCustomizableResponseType = findFile(request); - if (fileCustomizableResponseType != null) { - return runWithFilters(request, (filteredRequest, propagatedContext) - -> ExecutionFlow.just(HttpResponse.ok(fileCustomizableResponseType))); + UriRouteMatch routeMatch = routeExecutor.findRouteMatch(request); + if (routeMatch == null) { + if (validateUrl) { + try { + request.getUri(); // Invalid url will throw an exception + } catch (Throwable t) { + return onError(request, t.getCause()); + } + } + //Check if there is a file for the route before returning route not found + FileCustomizableResponseType fileCustomizableResponseType = findFile(request); + if (fileCustomizableResponseType != null) { + return runWithFilters(request, (filteredRequest, propagatedContext) + -> ExecutionFlow.just(HttpResponse.ok(fileCustomizableResponseType))); + } + return onRouteMiss(request); } - return onRouteMiss(request); - } - RouteExecutor.setRouteAttributes(request, routeMatch); + RouteExecutor.setRouteAttributes(request, routeMatch); - if (LOG.isTraceEnabled()) { - LOG.trace("Matched route {} - {} to controller {}", request.getMethodName(), request.getUri().getPath(), routeMatch.getDeclaringType()); - } - // all ok proceed to try and execute the route - if (routeMatch.getRouteInfo().isWebSocketRoute()) { - return onStatusError( - request, - HttpResponse.status(HttpStatus.BAD_REQUEST), - "Not a WebSocket request"); - } + if (LOG.isTraceEnabled()) { + LOG.trace("Matched route {} - {} to controller {}", request.getMethodName(), request.getUri().getPath(), routeMatch.getDeclaringType()); + } + // all ok proceed to try and execute the route + if (routeMatch.getRouteInfo().isWebSocketRoute()) { + return onStatusError( + request, + HttpResponse.status(HttpStatus.BAD_REQUEST), + "Not a WebSocket request"); + } - return runWithFilters(request, (filteredRequest, propagatedContext) -> executeRoute(filteredRequest, propagatedContext, routeMatch)); + return runWithFilters(request, (filteredRequest, propagatedContext) -> { + if (validateUrl) { + try { + request.getUri(); // Invalid url will throw an exception + } catch (Throwable t) { + return onError(filteredRequest, t.getCause()); + } + } + return executeRoute(filteredRequest, propagatedContext, routeMatch); + }); + } catch (Throwable t) { + return onError(request, t); + } } private ExecutionFlow> executeRoute(HttpRequest request, @@ -186,7 +206,12 @@ private ExecutionFlow> onErrorNoFilter(ExecutionFlow> onError(HttpRequest request, Throwable throwable) { - return runWithFilters(request, (filteredRequest, propagatedContext) -> onErrorNoFilter(filteredRequest, throwable, propagatedContext)); + try { + return runWithFilters(request, (filteredRequest, propagatedContext) -> onErrorNoFilter(filteredRequest, throwable, propagatedContext)) + .onErrorResume(t -> createDefaultErrorResponseFlow(request, t)); + } catch (Throwable e) { + return createDefaultErrorResponseFlow(request, e); + } } private ExecutionFlow> onErrorNoFilter(HttpRequest request, Throwable t, PropagatedContext propagatedContext) { diff --git a/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java b/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java index 6f8870bf594..1418c43150c 100644 --- a/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java +++ b/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java @@ -206,18 +206,22 @@ static void setRouteAttributes(HttpRequest request, UriRouteMatch createDefaultErrorResponse(HttpRequest httpRequest, Throwable cause) { logException(cause); - final MutableHttpResponse response = HttpResponse.serverError(); - response.setAttribute(HttpAttributes.EXCEPTION, cause); - response.setAttribute(HttpAttributes.ROUTE_INFO, new DefaultRouteInfo<>( + MutableHttpResponse mutableHttpResponse = HttpResponse.serverError(); + mutableHttpResponse.setAttribute(HttpAttributes.EXCEPTION, cause); + mutableHttpResponse.setAttribute(HttpAttributes.ROUTE_INFO, new DefaultRouteInfo<>( ReturnType.of(MutableHttpResponse.class, Argument.OBJECT_ARGUMENT), Object.class, true, false)); - MutableHttpResponse mutableHttpResponse = errorResponseProcessor.processResponse( - ErrorContext.builder(httpRequest) - .cause(cause) - .errorMessage("Internal Server Error: " + cause.getMessage()) - .build(), response); + try { + mutableHttpResponse = errorResponseProcessor.processResponse( + ErrorContext.builder(httpRequest) + .cause(cause) + .errorMessage("Internal Server Error: " + cause.getMessage()) + .build(), mutableHttpResponse); + } catch (Exception e) { + logException(e); + } applyConfiguredHeaders(mutableHttpResponse.getHeaders()); if (mutableHttpResponse.getContentType().isEmpty() && httpRequest.getMethod() != HttpMethod.HEAD) { return mutableHttpResponse.contentType(MediaType.APPLICATION_JSON_TYPE); diff --git a/http/src/main/java/io/micronaut/http/HttpHeaders.java b/http/src/main/java/io/micronaut/http/HttpHeaders.java index 47c2d5f2f2b..6ec6fb1d271 100644 --- a/http/src/main/java/io/micronaut/http/HttpHeaders.java +++ b/http/src/main/java/io/micronaut/http/HttpHeaders.java @@ -707,6 +707,16 @@ default List accept() { */ @Nullable default Charset acceptCharset() { + return findAcceptCharset().orElse(null); + } + + /** + * The {@code Accept-Charset} header, or empty if unset. + * + * @return The {@code Accept-Charset} header + * @since 4.3.0 + */ + default Optional findAcceptCharset() { return findFirst(HttpHeaders.ACCEPT_CHARSET) .map(text -> { text = HttpHeadersUtil.splitAcceptHeader(text); @@ -718,8 +728,7 @@ default Charset acceptCharset() { } // default to UTF-8 return StandardCharsets.UTF_8; - }) - .orElse(null); + }); } /** @@ -730,12 +739,21 @@ default Charset acceptCharset() { */ @Nullable default Locale acceptLanguage() { + return findAcceptLanguage().orElse(null); + } + + /** + * The {@code Accept-Language} header, or empty if unset. + * + * @return The {@code Accept-Language} header + * @since 4.3.0 + */ + default Optional findAcceptLanguage() { return findFirst(HttpHeaders.ACCEPT_LANGUAGE) .map(text -> { String part = HttpHeadersUtil.splitAcceptHeader(text); return part == null ? Locale.getDefault() : Locale.forLanguageTag(part); - }) - .orElse(null); + }); } /** diff --git a/http/src/main/java/io/micronaut/http/HttpRequest.java b/http/src/main/java/io/micronaut/http/HttpRequest.java index 3ca5a95cade..a684db8b243 100644 --- a/http/src/main/java/io/micronaut/http/HttpRequest.java +++ b/http/src/main/java/io/micronaut/http/HttpRequest.java @@ -169,7 +169,7 @@ default HttpRequest setAttribute(CharSequence name, Object value) { @Override default Optional getLocale() { - return Optional.ofNullable(getHeaders().acceptLanguage()); + return getHeaders().findAcceptLanguage(); } /** diff --git a/http/src/main/java/io/micronaut/http/simple/SimpleHttpRequest.java b/http/src/main/java/io/micronaut/http/simple/SimpleHttpRequest.java index 70e466762e5..dc84340c508 100644 --- a/http/src/main/java/io/micronaut/http/simple/SimpleHttpRequest.java +++ b/http/src/main/java/io/micronaut/http/simple/SimpleHttpRequest.java @@ -45,7 +45,7 @@ public class SimpleHttpRequest implements MutableHttpRequest { private final SimpleHttpHeaders headers = new SimpleHttpHeaders(ConversionService.SHARED); private final SimpleHttpParameters parameters = new SimpleHttpParameters(ConversionService.SHARED); - private HttpMethod method; + private final HttpMethod method; private URI uri; private Object body; @@ -53,13 +53,13 @@ public class SimpleHttpRequest implements MutableHttpRequest { * Simple {@link MutableHttpRequest} implementation. * * @param method the HTTP method - * @param uri the URI of the request + * @param url the URI of the request * @param body the optional body of the request */ - public SimpleHttpRequest(HttpMethod method, String uri, B body) { + public SimpleHttpRequest(HttpMethod method, String url, B body) { this.method = method; try { - this.uri = new URI(uri); + this.uri = new URI(url); } catch (URISyntaxException e) { throw new IllegalArgumentException("Wrong URI", e); } diff --git a/http/src/main/java/io/micronaut/http/util/HttpUtil.java b/http/src/main/java/io/micronaut/http/util/HttpUtil.java index a989c04632c..47f128feb9d 100644 --- a/http/src/main/java/io/micronaut/http/util/HttpUtil.java +++ b/http/src/main/java/io/micronaut/http/util/HttpUtil.java @@ -76,7 +76,7 @@ public static Optional resolveCharset(HttpMessage request) { if (contentTypeCharset.isPresent()) { return contentTypeCharset; } else { - return Optional.ofNullable(request.getHeaders().acceptCharset()); + return request.getHeaders().findAcceptCharset(); } } catch (UnsupportedCharsetException e) { return Optional.empty(); From b68f972bfc72d6621767c744d0c3da477b4e8960 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Tue, 5 Dec 2023 12:09:46 +0100 Subject: [PATCH 2/4] Correct --- .../http/netty/AbstractNettyHttpRequest.java | 8 ++++---- .../io/micronaut/http/netty/NettyHttpHeaders.java | 13 ++++++------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java b/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java index 2f5536b9349..cf68e852470 100644 --- a/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java +++ b/http-netty/src/main/java/io/micronaut/http/netty/AbstractNettyHttpRequest.java @@ -47,7 +47,7 @@ public abstract class AbstractNettyHttpRequest extends DefaultAttributeMap im protected final io.netty.handler.codec.http.HttpRequest nettyRequest; protected final ConversionService conversionService; protected final HttpMethod httpMethod; - protected final String url; + protected final String unvalidatedUrl; protected final String httpMethodName; private URI uri; @@ -62,7 +62,7 @@ public abstract class AbstractNettyHttpRequest extends DefaultAttributeMap im public AbstractNettyHttpRequest(io.netty.handler.codec.http.HttpRequest nettyRequest, ConversionService conversionService) { this.nettyRequest = nettyRequest; this.conversionService = conversionService; - this.url = nettyRequest.uri(); + this.unvalidatedUrl = nettyRequest.uri(); this.httpMethodName = nettyRequest.method().name(); this.httpMethod = HttpMethod.parse(httpMethodName); } @@ -162,7 +162,7 @@ public URI getUri() { synchronized (this) { // double check u = this.uri; if (u == null) { - u = createURI(url); + u = createURI(unvalidatedUrl); this.uri = u; } } @@ -177,7 +177,7 @@ public String getPath() { synchronized (this) { // double check p = this.path; if (p == null) { - p = parsePath(url); + p = parsePath(unvalidatedUrl); this.path = p; } } diff --git a/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java b/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java index 06908e9c449..7fde4d2eebf 100644 --- a/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java +++ b/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java @@ -21,7 +21,6 @@ import io.micronaut.core.convert.ConversionService; import io.micronaut.core.type.MutableHeaders; import io.micronaut.http.HttpHeaderValues; -import io.micronaut.http.HttpHeaders; import io.micronaut.http.MediaType; import io.micronaut.http.MutableHttpHeaders; import io.micronaut.http.util.HttpHeadersUtil; @@ -169,17 +168,17 @@ public MutableHeaders set(CharSequence header, CharSequence value) { } private void onModify(CharSequence header) { - if (contentType != null && header.equals(HttpHeaders.CONTENT_TYPE)) { + if (contentType != null && HttpHeaderNames.CONTENT_TYPE.contentEqualsIgnoreCase(header)) { contentType = null; - } else if (contentLength != null && header.equals(HttpHeaders.CONTENT_LENGTH)) { + } else if (contentLength != null && HttpHeaderNames.CONTENT_LENGTH.contentEqualsIgnoreCase(header)) { contentLength = null; - } else if (accept != null && header.equals(HttpHeaders.ACCEPT)) { + } else if (accept != null && HttpHeaderNames.ACCEPT.contentEqualsIgnoreCase(header)) { accept = null; - } else if (acceptCharset != null && header.equals(HttpHeaders.ACCEPT_CHARSET)) { + } else if (acceptCharset != null && HttpHeaderNames.ACCEPT_CHARSET.contentEqualsIgnoreCase(header)) { acceptCharset = null; - } else if (acceptLanguage != null && header.equals(HttpHeaders.ACCEPT_LANGUAGE)) { + } else if (acceptLanguage != null && HttpHeaderNames.ACCEPT_LANGUAGE.contentEqualsIgnoreCase(header)) { acceptLanguage = null; - } else if (origin != null && header.equals(HttpHeaders.ORIGIN)) { + } else if (origin != null && HttpHeaderNames.ORIGIN.contentEqualsIgnoreCase(header)) { origin = null; } } From 4dae954043d50cd794e189f7d2b2bc26bd85eac2 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Tue, 5 Dec 2023 12:54:12 +0100 Subject: [PATCH 3/4] Improve --- .../http/netty/NettyHttpHeaders.java | 64 +++++++++++-------- .../http/netty/NettyMutableHttpResponse.java | 8 +-- .../netty/body/NettyWritableBodyWriter.java | 2 +- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java b/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java index 7fde4d2eebf..0c740ce15a1 100644 --- a/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java +++ b/http-netty/src/main/java/io/micronaut/http/netty/NettyHttpHeaders.java @@ -35,6 +35,7 @@ import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Base64; import java.util.Collection; @@ -155,7 +156,7 @@ public Optional findFirst(CharSequence name) { public MutableHttpHeaders add(CharSequence header, CharSequence value) { validateHeader(header, value); nettyHeaders.add(header, value); - onModify(header); + onModify(); return this; } @@ -163,24 +164,17 @@ public MutableHttpHeaders add(CharSequence header, CharSequence value) { public MutableHeaders set(CharSequence header, CharSequence value) { validateHeader(header, value); nettyHeaders.set(header, value); - onModify(header); + onModify(); return this; } - private void onModify(CharSequence header) { - if (contentType != null && HttpHeaderNames.CONTENT_TYPE.contentEqualsIgnoreCase(header)) { - contentType = null; - } else if (contentLength != null && HttpHeaderNames.CONTENT_LENGTH.contentEqualsIgnoreCase(header)) { - contentLength = null; - } else if (accept != null && HttpHeaderNames.ACCEPT.contentEqualsIgnoreCase(header)) { - accept = null; - } else if (acceptCharset != null && HttpHeaderNames.ACCEPT_CHARSET.contentEqualsIgnoreCase(header)) { - acceptCharset = null; - } else if (acceptLanguage != null && HttpHeaderNames.ACCEPT_LANGUAGE.contentEqualsIgnoreCase(header)) { - acceptLanguage = null; - } else if (origin != null && HttpHeaderNames.ORIGIN.contentEqualsIgnoreCase(header)) { - origin = null; - } + private void onModify() { + contentType = null; + contentLength = null; + accept = null; + acceptCharset = null; + acceptLanguage = null; + origin = null; } /** @@ -191,7 +185,6 @@ private void onModify(CharSequence header) { */ public void setUnsafe(CharSequence header, CharSequence value) { nettyHeaders.set(header, value); - onModify(header); } public static void validateHeader(CharSequence name, CharSequence value) { @@ -206,14 +199,14 @@ public static void validateHeader(CharSequence name, CharSequence value) { @Override public MutableHttpHeaders remove(CharSequence header) { nettyHeaders.remove(header); - onModify(header); + onModify(); return this; } @Override public MutableHttpHeaders date(LocalDateTime date) { if (date != null) { - add(HttpHeaderNames.DATE, ZonedDateTime.of(date, ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.DATE, ZonedDateTime.of(date, ZoneId.systemDefault())); } return this; } @@ -221,7 +214,7 @@ public MutableHttpHeaders date(LocalDateTime date) { @Override public MutableHttpHeaders expires(LocalDateTime date) { if (date != null) { - add(HttpHeaderNames.EXPIRES, ZonedDateTime.of(date, ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.EXPIRES, ZonedDateTime.of(date, ZoneId.systemDefault())); } return this; } @@ -229,7 +222,7 @@ public MutableHttpHeaders expires(LocalDateTime date) { @Override public MutableHttpHeaders lastModified(LocalDateTime date) { if (date != null) { - add(HttpHeaderNames.LAST_MODIFIED, ZonedDateTime.of(date, ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.LAST_MODIFIED, ZonedDateTime.of(date, ZoneId.systemDefault())); } return this; } @@ -237,35 +230,39 @@ public MutableHttpHeaders lastModified(LocalDateTime date) { @Override public MutableHttpHeaders ifModifiedSince(LocalDateTime date) { if (date != null) { - add(HttpHeaderNames.IF_MODIFIED_SINCE, ZonedDateTime.of(date, ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.IF_MODIFIED_SINCE, ZonedDateTime.of(date, ZoneId.systemDefault())); } return this; } @Override public MutableHttpHeaders date(long timeInMillis) { - add(HttpHeaderNames.DATE, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.DATE, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); return this; } @Override public MutableHttpHeaders expires(long timeInMillis) { - add(HttpHeaderNames.EXPIRES, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.EXPIRES, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); return this; } @Override public MutableHttpHeaders lastModified(long timeInMillis) { - add(HttpHeaderNames.LAST_MODIFIED, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.LAST_MODIFIED, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); return this; } @Override public MutableHttpHeaders ifModifiedSince(long timeInMillis) { - add(HttpHeaderNames.IF_MODIFIED_SINCE, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); + setUnsafe(HttpHeaderNames.IF_MODIFIED_SINCE, ZonedDateTime.ofInstant(Instant.ofEpochMilli(timeInMillis), ZoneId.systemDefault())); return this; } + private void setUnsafe(CharSequence header, ZonedDateTime value) { + setUnsafe(header, value.withZoneSameInstant(GMT).format(DateTimeFormatter.RFC_1123_DATE_TIME)); + } + @Override public MutableHttpHeaders auth(String userInfo) { StringBuilder sb = new StringBuilder(); @@ -285,12 +282,22 @@ public MutableHttpHeaders allowGeneric(Collection method @Override public MutableHttpHeaders location(URI uri) { - return add(HttpHeaderNames.LOCATION, uri.toString()); + setUnsafe(HttpHeaderNames.LOCATION, uri.toString()); + return this; } @Override public MutableHttpHeaders contentType(MediaType mediaType) { - return add(HttpHeaderNames.CONTENT_TYPE, mediaType); + if (mediaType == null) { + nettyHeaders.remove(HttpHeaderNames.CONTENT_TYPE); + } else { + // optimization for content type validation + mediaType.validate(() -> NettyHttpHeaders.validateHeader(HttpHeaderNames.CONTENT_TYPE, mediaType)); + nettyHeaders.set(HttpHeaderNames.CONTENT_TYPE, mediaType); + } + contentType = Optional.ofNullable(mediaType); + return this; + } @Override @@ -424,4 +431,5 @@ private Optional resolveOrigin() { } return cachedOrigin; } + } diff --git a/http-netty/src/main/java/io/micronaut/http/netty/NettyMutableHttpResponse.java b/http-netty/src/main/java/io/micronaut/http/netty/NettyMutableHttpResponse.java index 270311195cb..a48de03c50a 100644 --- a/http-netty/src/main/java/io/micronaut/http/netty/NettyMutableHttpResponse.java +++ b/http-netty/src/main/java/io/micronaut/http/netty/NettyMutableHttpResponse.java @@ -316,13 +316,7 @@ public MutableHttpResponse body(@Nullable T body) { @Override public MutableHttpResponse contentType(MediaType mediaType) { - if (mediaType == null) { - headers.remove(HttpHeaderNames.CONTENT_TYPE); - } else { - // optimization for content type validation - mediaType.validate(() -> NettyHttpHeaders.validateHeader(HttpHeaderNames.CONTENT_TYPE, mediaType)); - headers.setUnsafe(HttpHeaderNames.CONTENT_TYPE, mediaType); - } + headers.contentType(mediaType); return this; } diff --git a/http-netty/src/main/java/io/micronaut/http/netty/body/NettyWritableBodyWriter.java b/http-netty/src/main/java/io/micronaut/http/netty/body/NettyWritableBodyWriter.java index 7c56c31a8c6..64e991db2f3 100644 --- a/http-netty/src/main/java/io/micronaut/http/netty/body/NettyWritableBodyWriter.java +++ b/http-netty/src/main/java/io/micronaut/http/netty/body/NettyWritableBodyWriter.java @@ -75,7 +75,7 @@ public void writeTo(HttpRequest request, MutableHttpResponse outgoi ByteBuf byteBuf = nettyContext.alloc().ioBuffer(128); MutableHttpHeaders outgoingHeaders = outgoingResponse.getHeaders(); if (mediaType != null && !outgoingHeaders.contains(HttpHeaders.CONTENT_TYPE)) { - outgoingHeaders.set(HttpHeaders.CONTENT_TYPE, mediaType); + outgoingHeaders.contentType(mediaType); } try (ByteBufOutputStream outputStream = new ByteBufOutputStream(byteBuf)) { DefaultFullHttpResponse fullHttpResponse = new DefaultFullHttpResponse( From 4ccf8ef1df38ce7ca4562ee352e6cce7f71d975a Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Tue, 5 Dec 2023 13:42:29 +0100 Subject: [PATCH 4/4] Bypass `HateoasErrorResponseProcessor` error --- .../exceptions/response/HateoasErrorResponseProcessor.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/http-server/src/main/java/io/micronaut/http/server/exceptions/response/HateoasErrorResponseProcessor.java b/http-server/src/main/java/io/micronaut/http/server/exceptions/response/HateoasErrorResponseProcessor.java index de7a8e2b893..7f205ab788e 100644 --- a/http-server/src/main/java/io/micronaut/http/server/exceptions/response/HateoasErrorResponseProcessor.java +++ b/http-server/src/main/java/io/micronaut/http/server/exceptions/response/HateoasErrorResponseProcessor.java @@ -66,7 +66,12 @@ public MutableHttpResponse processResponse(@NonNull ErrorContext erro } error.embedded("errors", errors); } - error.link(Link.SELF, Link.of(errorContext.getRequest().getUri())); + try { + error.link(Link.SELF, Link.of(errorContext.getRequest().getUri())); + } catch (Exception e) { + // Invalid URL + error.link(Link.SELF, Link.of(errorContext.getRequest().getPath())); + } return response.body(error).contentType(MediaType.APPLICATION_JSON_TYPE); }