From 60aa2b7db8cbec25722f8cfbc609cc5bc3d3792f Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Thu, 5 May 2022 11:41:59 +0300 Subject: [PATCH] Harmonize the usage of charset in the Content-Type headers for RESTEasy Reactive The idea is that we know only append it for specific response media types. Before this change, the charset was added depending on how the return type of the Resource method Relates to: #25295 --- .../basic/ClassLevelMediaTypeTest.java | 2 +- .../resource/basic/MatchedResourceTest.java | 2 +- .../server/core/EncodedMediaType.java | 35 +++++- .../serialization/DynamicEntityWriter.java | 3 +- .../vertx/test/mediatype/CharsetTest.java | 111 ++++++++++++++++++ .../basic/ClassLevelMediaTypeTest.java | 2 +- .../resource/basic/MatchedResourceTest.java | 2 +- 7 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/mediatype/CharsetTest.java diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/ClassLevelMediaTypeTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/ClassLevelMediaTypeTest.java index f949976fe9444..2ec2baaa7254a 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/ClassLevelMediaTypeTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/ClassLevelMediaTypeTest.java @@ -55,7 +55,7 @@ public void testApplicationJsonMediaType() { Response response = base.request().get(); Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); String body = response.readEntity(String.class); - Assertions.assertEquals(response.getHeaderString("Content-Type"), "application/json"); + Assertions.assertEquals(response.getHeaderString("Content-Type"), "application/json;charset=UTF-8"); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/MatchedResourceTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/MatchedResourceTest.java index deefb36c25a46..c20cd298b208e 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/MatchedResourceTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/MatchedResourceTest.java @@ -88,7 +88,7 @@ public void testMatch() throws Exception { WebTarget base = client.target(generateURL("/match")); Response response = base.request().header("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8") .get(); - Assertions.assertEquals("text/html", response.getHeaders().getFirst("Content-Type")); + Assertions.assertEquals("text/html;charset=UTF-8", response.getHeaders().getFirst("Content-Type")); String res = response.readEntity(String.class); Assertions.assertEquals("*/*", res, "Wrong response content"); response.close(); diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/EncodedMediaType.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/EncodedMediaType.java index c61541ce96268..4613525e18f99 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/EncodedMediaType.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/EncodedMediaType.java @@ -1,20 +1,46 @@ package org.jboss.resteasy.reactive.server.core; +import java.nio.charset.StandardCharsets; +import java.util.Objects; import javax.ws.rs.core.MediaType; import org.jboss.resteasy.reactive.server.spi.ContentType; /** * Wrapper around MediaType that saves the toString value, to avoid * the expensive header delegate processing. + * It also harmonizes the use of charset */ public class EncodedMediaType implements ContentType { final MediaType mediaType; + final String charset; String encoded; - String charset; public EncodedMediaType(MediaType mediaType) { - this.mediaType = mediaType; - this.charset = mediaType.getParameters().get("charset"); + MediaType effectiveMediaType = mediaType; + String effectiveCharset; + String originalCharset = mediaType.getParameters().get("charset"); + if (isStringMediaType(mediaType)) { + effectiveCharset = originalCharset; + if (effectiveCharset == null) { + effectiveCharset = StandardCharsets.UTF_8.name(); + } + } else { + // it doesn't make sense to add charset to non string types + effectiveCharset = null; + } + this.charset = effectiveCharset; + if (!Objects.equals(originalCharset, effectiveCharset)) { + effectiveMediaType = mediaType.withCharset(effectiveCharset); + } + this.mediaType = effectiveMediaType; + } + + // TODO: does this need to be more complex? + private boolean isStringMediaType(MediaType mediaType) { + String type = mediaType.getType(); + String subtype = mediaType.getSubtype(); + return (type.equals("application") && (subtype.contains("json") || subtype.contains("xml") || subtype.contains("yaml"))) + || type.equals("text"); } @Override @@ -37,9 +63,6 @@ public String getEncoded() { @Override public String getCharset() { - if (charset == null) { - return charset = mediaType.getParameters().get("charset"); - } return charset; } } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/serialization/DynamicEntityWriter.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/serialization/DynamicEntityWriter.java index 83d318850d2bd..4109b8e1182a7 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/serialization/DynamicEntityWriter.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/serialization/DynamicEntityWriter.java @@ -91,7 +91,8 @@ public void write(ResteasyReactiveRequestContext context, Object entity) throws serverSerializersMediaType = selectedMediaType; context.setResponseContentType(selectedMediaType); // this will be used as the fallback if Response does NOT contain a type - context.serverResponse().addResponseHeader(HttpHeaders.CONTENT_TYPE, selectedMediaType.toString()); + context.serverResponse().addResponseHeader(HttpHeaders.CONTENT_TYPE, + context.getResponseContentType().toString()); } } } else { diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/mediatype/CharsetTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/mediatype/CharsetTest.java new file mode 100644 index 0000000000000..c21a45dcae2c0 --- /dev/null +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/mediatype/CharsetTest.java @@ -0,0 +1,111 @@ +package org.jboss.resteasy.reactive.server.vertx.test.mediatype; + +import static io.restassured.RestAssured.when; +import static org.junit.jupiter.api.Assertions.*; + +import java.nio.charset.StandardCharsets; +import java.util.function.Supplier; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Response; +import org.jboss.resteasy.reactive.server.vertx.test.framework.ResteasyReactiveUnitTest; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public class CharsetTest { + + @RegisterExtension + static ResteasyReactiveUnitTest test = new ResteasyReactiveUnitTest() + .setArchiveProducer(new Supplier<>() { + @Override + public JavaArchive get() { + return ShrinkWrap.create(JavaArchive.class) + .addClasses(TestResource.class); + } + }); + + @Test + public void testText() { + String contentType = when().get("/test/text") + .then() + .statusCode(200) + .extract().header("Content-Type"); + assertEquals("text/plain;charset=UTF-8", contentType); + } + + @Test + public void testResponseText() { + String contentType = when().get("/test/response/text") + .then() + .statusCode(200) + .extract().header("Content-Type"); + assertEquals("text/plain;charset=UTF-8", contentType); + } + + @Test + public void testJson() { + String contentType = when().get("/test/json") + .then() + .statusCode(200) + .extract().header("Content-Type"); + assertEquals("application/json;charset=UTF-8", contentType); + } + + @Test + public void testImage() { + String contentType = when().get("/test/image") + .then() + .statusCode(200) + .extract().header("Content-Type"); + assertEquals("image/png", contentType); + } + + @Path("test") + public static class TestResource { + + @Path("text") + @Produces("text/plain") + @GET + public String textPlain() { + return "text"; + } + + @Path("response/text") + @Produces("text/plain") + @GET + public Response responseTextPlain() { + return Response.ok("text").build(); + } + + @Path("json") + @Produces("application/json") + @GET + public String json() { + return "{\"foo\": \"bar\"}"; + } + + @Path("response/json") + @Produces("application/json") + @GET + public Response responseJson() { + return Response.ok("{\"foo\": \"bar\"}").build(); + } + + @Path("image") + @Produces("image/png") + @GET + public Response imagePng() { + return Response.ok("fake image".getBytes(StandardCharsets.UTF_8)).build(); + } + + @Path("response/image") + @Produces("image/png") + @GET + public byte[] responseImagePng() { + return "fake image".getBytes(StandardCharsets.UTF_8); + } + } +} diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/ClassLevelMediaTypeTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/ClassLevelMediaTypeTest.java index 3937fba4a2c46..a302b4673d8b2 100644 --- a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/ClassLevelMediaTypeTest.java +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/ClassLevelMediaTypeTest.java @@ -52,7 +52,7 @@ public void testApplicationJsonMediaType() { Response response = base.request().get(); Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); String body = response.readEntity(String.class); - Assertions.assertEquals(response.getHeaderString("Content-Type"), "application/json"); + Assertions.assertEquals(response.getHeaderString("Content-Type"), "application/json;charset=UTF-8"); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/MatchedResourceTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/MatchedResourceTest.java index 5f3dd4ad52250..63cd7fad61906 100644 --- a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/MatchedResourceTest.java +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/MatchedResourceTest.java @@ -85,7 +85,7 @@ public void testMatch() throws Exception { WebTarget base = client.target(generateURL("/match")); Response response = base.request().header("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8") .get(); - Assertions.assertEquals("text/html", response.getHeaders().getFirst("Content-Type")); + Assertions.assertEquals("text/html;charset=UTF-8", response.getHeaders().getFirst("Content-Type")); String res = response.readEntity(String.class); Assertions.assertEquals("*/*", res, "Wrong response content"); response.close();