From 50c9b9b1d30b011ee8cc597da4cd9a0358ed5262 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 +- .../server/core/EncodedMediaType.java | 32 ++++- .../serialization/DynamicEntityWriter.java | 3 +- .../vertx/test/mediatype/CharsetTest.java | 111 ++++++++++++++++++ .../basic/ClassLevelMediaTypeTest.java | 2 +- 5 files changed, 141 insertions(+), 9 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 f949976fe94440..2ec2baaa7254aa 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/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 c61541ce96268c..6fb9f78eea3464 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,5 +1,7 @@ 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; @@ -9,12 +11,33 @@ */ 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) { + return mediaType.getType().equals("application") + || (mediaType.getType().equals("text") && mediaType.getSubtype().equals("plain")); } @Override @@ -37,9 +60,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 83d318850d2bd4..4109b8e1182a7b 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 00000000000000..c21a45dcae2c0f --- /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 3937fba4a2c462..a302b4673d8b20 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); }