Skip to content

Commit

Permalink
fix: Error Page Key should consider error messages (#11448)
Browse files Browse the repository at this point in the history 
* fix: Error Page Key should consider error messages

Close: #11447

* javadoc: use {@code

* javadoc: correct constructor reference

* revert changes

* remove unnecessary DefaultHtmlErrorResponseBodyProvider.HtmlErrorPage
  • Loading branch information
@sdelamo
sdelamo authored Dec 17, 2024
1 parent 40478d2 commit c12c1ea
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 27 deletions.
87 changes: 87 additions & 0 deletions ...rc/test/java/io/micronaut/http/server/exceptions/response/HtmlErrorResponseCacheTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package io.micronaut.http.server.exceptions.response;

import io.micronaut.context.annotation.Property;
import io.micronaut.context.annotation.Requires;
import io.micronaut.core.type.Argument;
import io.micronaut.http.*;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.annotation.Produces;
import io.micronaut.http.client.BlockingHttpClient;
import io.micronaut.http.client.HttpClient;
import io.micronaut.http.client.annotation.Client;
import io.micronaut.http.client.exceptions.HttpClientResponseException;
import io.micronaut.http.uri.UriBuilder;
import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
import org.junit.jupiter.api.Test;

import java.net.URI;
import java.util.Optional;

import static org.junit.jupiter.api.Assertions.*;

@Property(name = "spec.name", value = "HtmlErrorResponseCacheTest")
@MicronautTest
class HtmlErrorResponseCacheTest {

@Test
void cache(@Client("/") HttpClient httpClient) {
BlockingHttpClient client = httpClient.toBlocking();

URI uri = UriBuilder.of("/example").build();
HttpRequest<?> request = HttpRequest.GET(uri).accept(MediaType.TEXT_HTML);
Argument<String> arg = Argument.of(String.class);
HttpClientResponseException ex = assertThrows(HttpClientResponseException.class, () -> client.exchange(request, arg, arg));
assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus());
Optional<String> htmlOptional = ex.getResponse().getBody(String.class);
assertTrue(htmlOptional.isPresent());
String html = htmlOptional.get();
assertTrue(html.contains("<!doctype html>"));
assertTrue(html.contains("Required argument [String a] not specified"));
assertFalse(html.contains("Required argument [String b] not specified"));

ex = assertThrows(HttpClientResponseException.class, () -> client.exchange(
HttpRequest.GET(UriBuilder.of("/example").queryParam("a", "foo").build())
.accept(MediaType.TEXT_HTML),
arg,
arg));
assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus());
htmlOptional = ex.getResponse().getBody(String.class);
assertTrue(htmlOptional.isPresent());
html = htmlOptional.get();
assertTrue(html.contains("<!doctype html>"));
assertFalse(html.contains("Required argument [String a] not specified"));
assertTrue(html.contains("Required argument [String b] not specified"));

HttpResponse<?> response = assertDoesNotThrow(() -> client.exchange(
HttpRequest.GET(UriBuilder.of("/example")
.queryParam("a", "foo")
.queryParam("b", "bar")
.build())
.accept(MediaType.TEXT_HTML),
arg,
arg));
assertEquals(HttpStatus.OK, response.getStatus());
htmlOptional = response.getBody(String.class);
assertTrue(htmlOptional.isPresent());
}

@Requires(property = "spec.name", value = "HtmlErrorResponseCacheTest")
@Controller("/example")
static class ExampleController {
@Produces({ MediaType.TEXT_HTML})
@Get
public String index(String a, String b) {
return """
<!DOCTYPE html>
<html>
<head>
<title>Hello World</title>
</head>
<body>
</body>
</html>
""";
}
}
}
2 changes: 1 addition & 1 deletion http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ public void setMaxRequestSize(@ReadableBytes long maxRequestSize) {
* Sets the maximum number of request bytes that will be buffered. Fully streamed requests can
* still exceed this value. Default value ({@value #DEFAULT_MAX_REQUEST_BUFFER_SIZE} =&gt; // 10MB).
* Currently limited to {@code 2^31}, if you need longer request bodies, stream them.<br>
* Note that there is always some internal buffering, so a very low value (< ~64K) will
* Note that there is always some internal buffering, so a very low value ({@code < ~64K}) will
* essentially act like a request size limit.
*
* @param maxRequestBufferSize The maximum number of bytes from the request that may be buffered if the application requests buffering
Expand Down
2 changes: 1 addition & 1 deletion http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public class CorsFilter implements Ordered, ConditionalFilter {
/**
* @param corsConfiguration The {@link CorsOriginConfiguration} instance
* @param httpHostResolver HTTP Host resolver
* @deprecated use {@link CorsFilter(HttpServerConfiguration, HttpHostResolver, Router)} instead.
* @deprecated use {@link CorsFilter(HttpServerConfiguration.CorsConfiguration, HttpHostResolver, Router)} instead.
*/
@Deprecated(since = "4.7", forRemoval = true)
public CorsFilter(HttpServerConfiguration.CorsConfiguration corsConfiguration,
Expand Down
66 changes: 41 additions & 25 deletions ...va/io/micronaut/http/server/exceptions/response/DefaultHtmlErrorResponseBodyProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import jakarta.inject.Singleton;

import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -128,7 +130,7 @@ final class DefaultHtmlErrorResponseBodyProvider implements HtmlErrorResponseBod
private final HtmlSanitizer htmlSanitizer;
private final MessageSource messageSource;
private final LocaleResolver<HttpRequest<?>> localeResolver;
private final Map<LocaleStatus, String> cache = new ConcurrentHashMap<>();
private final Map<HtmlErrorPage, String> cache = new ConcurrentHashMap<>();

DefaultHtmlErrorResponseBodyProvider(HtmlSanitizer htmlSanitizer,
MessageSource messageSource,
Expand All @@ -140,47 +142,57 @@ final class DefaultHtmlErrorResponseBodyProvider implements HtmlErrorResponseBod

@Override
public String body(@NonNull ErrorContext errorContext, @NonNull HttpResponse<?> response) {
int httpStatusCode = response.code();
String httpStatusReason = htmlSanitizer.sanitize(response.reason());
Locale locale = localeResolver.resolveOrDefault(errorContext.getRequest());
return cache.computeIfAbsent(new LocaleStatus(locale, httpStatusCode), key -> html(locale, httpStatusCode, httpStatusReason, errorContext));
HtmlErrorPage key = error(errorContext, response);
return cache.computeIfAbsent(key, this::html);
}

private String html(Locale locale,
int httpStatusCode,
String httpStatusReason,
ErrorContext errorContext) {
final String errorTitleCode = httpStatusCode + ".error.title";
final String errorTitle = messageSource.getMessage(errorTitleCode, httpStatusReason, locale);
private String html(@NonNull HtmlErrorPage htmlErrorPage) {
final String errorTitleCode = htmlErrorPage.httpStatusCode() + ".error.title";
final String errorTitle = messageSource.getMessage(errorTitleCode, htmlErrorPage.httpStatusReason(), htmlErrorPage.locale());
String header = "<h1>" + errorTitle + "</h1>";
header += "<h2>" + httpStatusCode + "</h1>";
header += "<h2>" + htmlErrorPage.httpStatusCode() + "</h1>";
return MessageFormat.format("<!doctype html><html lang=\"en\"><head><title>{0} — {1}</title><meta charset=\"utf-8\"><meta name=\"viewport\" content=\"initial-scale=1, width=device-width\"><meta name=\"robots\" content=\"noindex, nofollow\"><style>{2}</style></head><body><main><header>{3}</header><article>{4}</article></main></body></html>",
httpStatusCode,
htmlErrorPage.httpStatusCode(),
errorTitle,
CSS,
header,
article(locale, httpStatusCode, httpStatusReason, errorContext));
article(htmlErrorPage));
}

private String article(Locale locale,
int httpStatusCode,
String httpStatusReason,
ErrorContext errorContext) {
private HtmlErrorPage error(@NonNull ErrorContext errorContext,
@NonNull HttpResponse<?> response) {
int httpStatusCode = response.code();
Locale locale = localeResolver.resolveOrDefault(errorContext.getRequest());
final String errorBoldCode = httpStatusCode + ".error.bold";
final String errorCode = httpStatusCode + ".error";
String defaultErrorBold = DEFAULT_ERROR_BOLD.get(httpStatusCode);
String defaultError = DEFAULT_ERROR.get(httpStatusCode);
String errorBold = defaultErrorBold != null ? messageSource.getMessage(errorBoldCode, defaultErrorBold, locale) : messageSource.getMessage(errorBoldCode, locale).orElse(null);
String error = defaultError != null ? messageSource.getMessage(errorCode, defaultError, locale) : messageSource.getMessage(errorCode, locale).orElse(null);
StringBuilder sb = new StringBuilder();
String errorBold = defaultErrorBold != null
? messageSource.getMessage(errorBoldCode, defaultErrorBold, locale)
: messageSource.getMessage(errorBoldCode, locale).orElse(null);
String error = defaultError != null
? messageSource.getMessage(errorCode, defaultError, locale)
: messageSource.getMessage(errorCode, locale).orElse(null);
String httpStatusReason = htmlSanitizer.sanitize(response.reason());

List<String> messages = new ArrayList<>();
for (io.micronaut.http.server.exceptions.response.Error e : errorContext.getErrors()) {
if (!e.getMessage().equalsIgnoreCase(httpStatusReason)) {
sb.append(htmlSanitizer.sanitize(e.getMessage()));
sb.append("<br/>");
messages.add(htmlSanitizer.sanitize(e.getMessage()));
}
}
return new HtmlErrorPage(locale, httpStatusCode, httpStatusReason, error, errorBold, messages);
}

private String article(@NonNull HtmlErrorPage htmlErrorPage) {
StringBuilder sb = new StringBuilder();

for (String message : htmlErrorPage.messages) {
sb.append(message);
sb.append("<br/>");
}
String error = htmlErrorPage.error();
String errorBold = htmlErrorPage.errorBold();
if (error != null || errorBold != null) {
sb.append("<p>");
if (errorBold != null) {
Expand All @@ -197,7 +209,11 @@ private String article(Locale locale,
return sb.toString();
}

private record LocaleStatus(Locale locale, int httpStatusCode) {

private record HtmlErrorPage(Locale locale,
int httpStatusCode,
String httpStatusReason,
String error,
String errorBold,
List<String> messages) {
}
}

0 comments on commit c12c1ea

Please sign in to comment.