This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository 
https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git

commit 6f74d08defb2a1b4ed0661614de27f8ca3b85b25
Author: yhs0092 <yaohai...@huawei.com>
AuthorDate: Wed May 16 15:03:06 2018 +0800

    [SCB-580] wrap failure response body as json string
---
 .../transport/rest/vertx/VertxRestDispatcher.java  | 30 +++++++++++++---
 .../rest/vertx/TestVertxRestDispatcher.java        | 41 ++++++++++++++++++++--
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git 
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/VertxRestDispatcher.java
 
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/VertxRestDispatcher.java
index 3589370..0d0c0e6 100644
--- 
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/VertxRestDispatcher.java
+++ 
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/VertxRestDispatcher.java
@@ -37,6 +37,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import 
io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.ErrorDataDecoderException;
+import io.vertx.core.json.JsonObject;
 import io.vertx.ext.web.Router;
 import io.vertx.ext.web.RoutingContext;
 import io.vertx.ext.web.handler.CookieHandler;
@@ -132,14 +133,35 @@ public class VertxRestDispatcher extends 
AbstractVertxHttpDispatcher {
   }
 
   /**
-   * Consumer will treat the response body as json by default, so it's 
necessary to wrap response body with ""
+   * Consumer will treat the response body as json by default, so it's 
necessary to wrap response body as Json string
    * to avoid deserialization error.
    *
    * @param message response body
-   * @return response body wrapped with ""
+   * @return response body wrapped as Json string
    */
-  private String wrapResponseBody(String message) {
-    return "\"" + message + "\"";
+  String wrapResponseBody(String message) {
+    if (isValidJson(message)) {
+      return message;
+    }
+
+    JsonObject jsonObject = new JsonObject();
+    jsonObject.put("message", message);
+
+    return jsonObject.toString();
+  }
+
+  /**
+   * Check if the message is a valid Json string.
+   * @param message the message to be checked.
+   * @return true if message is a valid Json string, otherwise false.
+   */
+  private boolean isValidJson(String message) {
+    try {
+      new JsonObject(message);
+    } catch (Exception ignored) {
+      return false;
+    }
+    return true;
   }
 
   /**
diff --git 
a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java
 
b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java
index 4188bee..059a7fb 100644
--- 
a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java
+++ 
b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestVertxRestDispatcher.java
@@ -50,6 +50,7 @@ import io.vertx.core.http.HttpMethod;
 import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.http.HttpServerResponse;
 import io.vertx.core.impl.VertxImpl;
+import io.vertx.core.json.JsonObject;
 import io.vertx.core.net.SocketAddress;
 import io.vertx.ext.web.Router;
 import io.vertx.ext.web.RoutingContext;
@@ -195,7 +196,7 @@ public class TestVertxRestDispatcher {
     Assert.assertThat(response.responseStatusCode, 
Matchers.is(Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode()));
     Assert.assertThat(response.responseStatusMessage, 
Matchers.is(Status.REQUEST_ENTITY_TOO_LARGE.getReasonPhrase()));
     Assert.assertThat(response.responseChunk,
-        Matchers.is("\"" + Status.REQUEST_ENTITY_TOO_LARGE.getReasonPhrase() + 
"\""));
+        Matchers.is("{\"message\":\"" + 
Status.REQUEST_ENTITY_TOO_LARGE.getReasonPhrase() + "\"}"));
     Assert.assertTrue(response.responseEnded);
   }
 
@@ -220,7 +221,7 @@ public class TestVertxRestDispatcher {
     Assert.assertThat(response.responseHeader, 
Matchers.hasEntry(HttpHeaders.CONTENT_TYPE, MediaType.WILDCARD));
     Assert.assertThat(response.responseStatusCode, 
Matchers.is(Status.INTERNAL_SERVER_ERROR.getStatusCode()));
     Assert.assertThat(response.responseChunk,
-        Matchers.is("\"" + exceptionMessage + "\""));
+        Matchers.is("{\"message\":\"" + exceptionMessage + "\"}"));
     Assert.assertTrue(response.responseEnded);
   }
 
@@ -269,7 +270,7 @@ public class TestVertxRestDispatcher {
     Assert.assertThat(response.responseStatusCode, 
Matchers.is(Status.INTERNAL_SERVER_ERROR.getStatusCode()));
     Assert.assertThat(response.responseStatusMessage, 
Matchers.is(Status.INTERNAL_SERVER_ERROR.getReasonPhrase()));
     Assert.assertThat(response.responseChunk,
-        Matchers.is("\"" + Status.INTERNAL_SERVER_ERROR.getReasonPhrase() + 
"\""));
+        Matchers.is("{\"message\":\"" + 
Status.INTERNAL_SERVER_ERROR.getReasonPhrase() + "\"}"));
     Assert.assertTrue(response.responseEnded);
   }
 
@@ -301,6 +302,40 @@ public class TestVertxRestDispatcher {
     Assert.assertEquals(RestProducerInvocation.class, 
map.get(RestConst.REST_PRODUCER_INVOCATION).getClass());
     Assert.assertTrue(invoked);
   }
+
+  @Test
+  public void testWrapResponseBody() {
+    VertxRestDispatcher vertxRestDispatcher = new VertxRestDispatcher();
+    String message = "abcd";
+    String bodyString = vertxRestDispatcher.wrapResponseBody(message);
+    Assert.assertNotNull(bodyString);
+    Assert.assertEquals("{\"message\":\"abcd\"}", bodyString);
+
+    message = "\"abcd\"";
+    bodyString = vertxRestDispatcher.wrapResponseBody(message);
+    Assert.assertNotNull(bodyString);
+    Assert.assertEquals("{\"message\":\"\\\"abcd\\\"\"}", bodyString);
+
+    message = ".01ab\"!@#$%^&*()'\\cd";
+    bodyString = vertxRestDispatcher.wrapResponseBody(message);
+    Assert.assertNotNull(bodyString);
+    Assert.assertEquals("{\"message\":\".01ab\\\"!@#$%^&*()'\\\\cd\"}", 
bodyString);
+
+    message = new JsonObject().put("key", new JsonObject().put("k2", 
"value")).toString();
+    bodyString = vertxRestDispatcher.wrapResponseBody(message);
+    Assert.assertNotNull(bodyString);
+    Assert.assertEquals("{\"key\":{\"k2\":\"value\"}}", bodyString);
+
+    message = "ab\"23\n@!#cd";
+    bodyString = vertxRestDispatcher.wrapResponseBody(message);
+    Assert.assertNotNull(bodyString);
+    Assert.assertEquals("{\"message\":\"ab\\\"23\\n@!#cd\"}", bodyString);
+
+    message = "ab\"23\r\n@!#cd";
+    bodyString = vertxRestDispatcher.wrapResponseBody(message);
+    Assert.assertNotNull(bodyString);
+    Assert.assertEquals("{\"message\":\"ab\\\"23\\r\\n@!#cd\"}", bodyString);
+  }
 }
 
 class MockHttpServerResponse implements HttpServerResponse {

-- 
To stop receiving notification emails like this one, please contact
liu...@apache.org.

Reply via email to