Wrap possible HTML plaintext in JSON

If the HTML appears like MSIE might guess it is HTML (such as if it
contains <) encode the response as a JSON object instead of as a
simple plain text string. This won't show up very often for clients,
and protects MSIE users stuck on ancient versions (pre MSIE 8).

Change-Id: I1fc32fda4093b6ad2f8f492f3457db5adaa906b4

gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java

@@ -65,12 +65,30 @@ public class RestApi {
     public void onResponseReceived(Request req, Response res) {
       int status = res.getStatusCode();
       if (status != 200) {
-        RpcStatus.INSTANCE.onRpcComplete();
+        String msg;
-        if ((400 <= status && status < 600) && isTextBody(res)) {
+        if (isTextBody(res)) {
-          cb.onFailure(new RemoteJsonException(res.getText(), status, null));
+          msg = res.getText().trim();
-        } else {
+        } else if (isJsonBody(res)) {
-          cb.onFailure(new StatusCodeException(status, res.getStatusText()));
+          try {
+            ErrorMessage error = parseJson(res);
+            msg = error.message() != null
+                ? error.message()
+                : res.getText().trim();
+          } catch (JSONException e) {
+            msg = res.getText().trim();
           }
+        } else {
+          msg = res.getStatusText();
+        }
+
+        Throwable error;
+        if (400 <= status && status < 600) {
+          error = new RemoteJsonException(msg, status, null);
+        } else {
+          error = new StatusCodeException(status, res.getStatusText());
+        }
+        RpcStatus.INSTANCE.onRpcComplete();
+        cb.onFailure(error);
         return;
       }
 
@@ -82,19 +100,9 @@ public class RestApi {
         return;
       }
 
-      String json = res.getText();
-      if (json.startsWith(JSON_MAGIC)) {
-        json = json.substring(JSON_MAGIC.length());
-      }
-      if (json.isEmpty()) {
-        RpcStatus.INSTANCE.onRpcComplete();
-        cb.onFailure(new RemoteJsonException("JSON response was empty"));
-        return;
-      }
-
       T data;
       try {
-        data = cast(JSONParser.parseStrict(json));
+        data = parseJson(res);
       } catch (JSONException e) {
         RpcStatus.INSTANCE.onRpcComplete();
         cb.onFailure(new RemoteJsonException("Invalid JSON: " + e.getMessage()));
@@ -105,21 +113,6 @@ public class RestApi {
       RpcStatus.INSTANCE.onRpcComplete();
     }
 
-    @SuppressWarnings("unchecked")
-    private T cast(JSONValue val) {
-      if (val.isObject() != null) {
-        return (T) val.isObject().getJavaScriptObject();
-      } else if (val.isArray() != null) {
-        return (T) val.isArray().getJavaScriptObject();
-      } else if (val.isString() != null) {
-        return (T) NativeString.wrap(val.isString().stringValue());
-      } else if (val.isNull() != null) {
-        return null;
-      } else {
-        throw new JSONException("Unsupported JSON response type");
-      }
-    }
-
     @Override
     public void onError(Request req, Throwable err) {
       RpcStatus.INSTANCE.onRpcComplete();
@@ -268,4 +261,38 @@ public class RestApi {
     }
     return want.equals(type);
   }
+
+  private static <T extends JavaScriptObject> T parseJson(Response res)
+      throws JSONException {
+    String json = res.getText();
+    if (json.startsWith(JSON_MAGIC)) {
+      json = json.substring(JSON_MAGIC.length());
+    }
+    if (json.isEmpty()) {
+      throw new JSONException("response was empty");
+    }
+    return cast(JSONParser.parseStrict(json));
+  }
+
+  @SuppressWarnings("unchecked")
+  private static <T extends JavaScriptObject> T cast(JSONValue val) {
+    if (val.isObject() != null) {
+      return (T) val.isObject().getJavaScriptObject();
+    } else if (val.isArray() != null) {
+      return (T) val.isArray().getJavaScriptObject();
+    } else if (val.isString() != null) {
+      return (T) NativeString.wrap(val.isString().stringValue());
+    } else if (val.isNull() != null) {
+      return null;
+    } else {
+      throw new JSONException("unsupported JSON type");
+    }
+  }
+
+  private static class ErrorMessage extends JavaScriptObject {
+    final native String message() /*-{ return this.message; }-*/;
+
+    protected ErrorMessage() {
+    }
+  }
 }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -28,6 +28,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
@@ -63,6 +64,7 @@ import com.google.gson.FieldNamingPolicy;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
 import com.google.gson.stream.JsonReader;
 import com.google.gson.stream.JsonToken;
@@ -93,6 +95,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.zip.GZIPOutputStream;
 
 import javax.annotation.Nullable;
@@ -388,9 +391,10 @@ public class RestApiServlet extends HttpServlet {
     return c.newInstance();
   }
 
-  private static void replyJson(HttpServletRequest req,
+  private static void replyJson(@Nullable HttpServletRequest req,
       HttpServletResponse res,
-      Multimap<String, String> config, Object result)
+      Multimap<String, String> config,
+      Object result)
       throws IOException {
     final TemporaryBuffer.Heap buf = heap(Integer.MAX_VALUE);
     buf.write(JSON_MAGIC);
@@ -421,7 +425,7 @@ public class RestApiServlet extends HttpServlet {
       FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES;
 
   private static Gson newGson(Multimap<String, String> config,
-      HttpServletRequest req) {
+      @Nullable HttpServletRequest req) {
     GsonBuilder gb = OutputFormat.JSON_COMPACT.newGsonBuilder()
       .setFieldNamingPolicy(NAMING);
 
@@ -432,11 +436,12 @@ public class RestApiServlet extends HttpServlet {
   }
 
   private static void enablePrettyPrint(GsonBuilder gb,
-      Multimap<String, String> config, HttpServletRequest req) {
+      Multimap<String, String> config,
+      @Nullable HttpServletRequest req) {
     String pp = Iterables.getFirst(config.get("pp"), null);
     if (pp == null) {
       pp = Iterables.getFirst(config.get("prettyPrint"), null);
-      if (pp == null) {
+      if (pp == null && req != null) {
         pp = acceptsJson(req) ? "0" : "1";
       }
     }
@@ -484,8 +489,10 @@ public class RestApiServlet extends HttpServlet {
     }
   }
 
-  private static void replyBinaryResult(HttpServletRequest req,
+  private static void replyBinaryResult(
-      HttpServletResponse res, BinaryResult bin) throws IOException {
+      @Nullable HttpServletRequest req,
+      HttpServletResponse res,
+      BinaryResult bin) throws IOException {
     try {
       res.setContentType(bin.getContentType());
       OutputStream dst = res.getOutputStream();
@@ -651,12 +658,23 @@ public class RestApiServlet extends HttpServlet {
 
   static void replyText(@Nullable HttpServletRequest req,
       HttpServletResponse res, String text) throws IOException {
+    if (isMaybeHTML(text)) {
+      JsonObject obj = new JsonObject();
+      obj.addProperty("message", text);
+      replyJson(req, res, ImmutableMultimap.of("pp", "0"), obj);
+    } else {
       if (!text.endsWith("\n")) {
         text += "\n";
       }
       replyBinaryResult(req, res,
           BinaryResult.create(text).setContentType("text/plain"));
     }
+  }
+
+  private static final Pattern IS_HTML = Pattern.compile("[<&]");
+  private static boolean isMaybeHTML(String text) {
+    return IS_HTML.matcher(text).find();
+  }
 
   private static boolean acceptsJson(HttpServletRequest req) {
     return req != null && isType(JSON_TYPE, req.getHeader("Accept"));