Merge pull request #25390 from geoand/#25295

Harmonize the usage of charset in the Content-Type headers for RESTEasy Reactive
quarkusio · May 16, 2022 · 9cf5ea2 · 9cf5ea2
2 parents 51a0642 + 60aa2b7
commit 9cf5ea2
Show file tree

Hide file tree

Showing 7 changed files with 146 additions and 11 deletions.
diff --git a/...java/io/quarkus/resteasy/reactive/server/test/resource/basic/ClassLevelMediaTypeTest.java b/...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/...est/java/io/quarkus/resteasy/reactive/server/test/resource/basic/MatchedResourceTest.java b/...est/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/...erver/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/EncodedMediaType.java b/...erver/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/.../main/java/org/jboss/resteasy/reactive/server/core/serialization/DynamicEntityWriter.java b/.../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/...tx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/mediatype/CharsetTest.java b/...tx/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/...org/jboss/resteasy/reactive/server/vertx/test/resource/basic/ClassLevelMediaTypeTest.java b/...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/...ava/org/jboss/resteasy/reactive/server/vertx/test/resource/basic/MatchedResourceTest.java b/...ava/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();