InlineEdit: Fix mangling of non-ASCII characters
atob() does not handle Unicode well. The decoding routine creates a
Unicode DOM string from the decoded bytes effectively treating the
stream as ISO-8859-1. This breaks non-ASCII by decomposing characters
into several meaningless "characters".
Instead of decoding base64 in the browser, modify the server to return
base64() BinaryResult as a JSON string if the only value of the Accept
request header is application/json. RestApi client code currently
always sets "Accept: application/json" in the request, as the browser
JS strongly prefers JSON in the response body.
Relying on the native JavaScript parser in the browser to read and
decode the JSON string is smaller and more efficient than a pure
JavaScript implementation of base64 decoding that supports UTF-8.
Fix the Content-Type used during POST and PUT to document charset is
UTF-8. The XMLHttpRequest standard requires browsers to always encode
in UTF-8, however the server examines the request's Content-Type
header to extract the charset attribute. Without setting the charset
an arbitrary default encoding was chosen by the server, which may still
mangle a commit message or file during save.
Bug: issue 3093
Change-Id: I549037bfb82aff15bbffc8b4a01e8955ae0ce318
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/BinaryResult.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/BinaryResult.java
index e264b31..18f356b 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/BinaryResult.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/BinaryResult.java
@@ -14,11 +14,17 @@
package com.google.gerrit.extensions.restapi;
+import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
-import java.io.UnsupportedEncodingException;
+import java.nio.ByteBuffer;
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.Charset;
+import java.nio.charset.CodingErrorAction;
+import java.nio.charset.StandardCharsets;
+import java.nio.charset.UnsupportedCharsetException;
/**
* Wrapper around a non-JSON result from a {@link RestView}.
@@ -34,13 +40,7 @@
/** Produce a UTF-8 encoded result from a string. */
public static BinaryResult create(String data) {
- try {
- return create(data.getBytes("UTF-8"))
- .setContentType("text/plain")
- .setCharacterEncoding("UTF-8");
- } catch (UnsupportedEncodingException e) {
- throw new RuntimeException("JVM does not support UTF-8", e);
- }
+ return new StringResult(data);
}
/** Produce an {@code application/octet-stream} result from a byte array. */
@@ -144,6 +144,28 @@
*/
public abstract void writeTo(OutputStream os) throws IOException;
+ /**
+ * Return a copy of the result as a String.
+ * <p>
+ * The default version of this method copies the result into a temporary byte
+ * array and then tries to decode it using the configured encoding.
+ *
+ * @return string version of the result.
+ * @throws IOException if the data cannot be produced or could not be
+ * decoded to a String.
+ */
+ public String asString() throws IOException {
+ long len = getContentLength();
+ ByteArrayOutputStream buf;
+ if (0 < len) {
+ buf = new ByteArrayOutputStream((int) len);
+ } else {
+ buf = new ByteArrayOutputStream();
+ }
+ writeTo(buf);
+ return decode(buf.toByteArray(), getCharacterEncoding());
+ }
+
/** Close the result and release any resources it holds. */
@Override
public void close() throws IOException {
@@ -161,6 +183,25 @@
getContentType());
}
+ private static String decode(byte[] data, String enc) {
+ try {
+ Charset cs = enc != null
+ ? Charset.forName(enc)
+ : StandardCharsets.UTF_8;
+ return cs.newDecoder()
+ .onMalformedInput(CodingErrorAction.REPORT)
+ .onUnmappableCharacter(CodingErrorAction.REPORT)
+ .decode(ByteBuffer.wrap(data))
+ .toString();
+ } catch (UnsupportedCharsetException | CharacterCodingException e) {
+ // Fallback to ISO-8850-1 style encoding.
+ StringBuilder r = new StringBuilder(data.length);
+ for (byte b : data)
+ r.append((char) (b & 0xff));
+ return r.toString();
+ }
+ }
+
private static class Array extends BinaryResult {
private final byte[] data;
@@ -173,6 +214,27 @@
public void writeTo(OutputStream os) throws IOException {
os.write(data);
}
+
+ @Override
+ public String asString() {
+ return decode(data, getCharacterEncoding());
+ }
+ }
+
+ private static class StringResult extends Array {
+ private final String str;
+
+ StringResult(String str) {
+ super(str.getBytes(StandardCharsets.UTF_8));
+ setContentType("text/plain");
+ setCharacterEncoding("UTF-8");
+ this.str = str;
+ }
+
+ @Override
+ public String asString() {
+ return str;
+ }
}
private static class Stream extends BinaryResult {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
index e87853b..e48477f 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
@@ -45,6 +45,7 @@
private static final String JSON_TYPE = "application/json";
private static final String JSON_UTF8 = JSON_TYPE + "; charset=utf-8";
private static final String TEXT_TYPE = "text/plain";
+ private static final String TEXT_UTF8 = TEXT_TYPE + "; charset=utf-8";
/**
* Expected JSON content body prefix that prevents XSSI.
@@ -129,9 +130,14 @@
final String type;
if (isJsonBody(res)) {
try {
- // javac generics bug
- data = RestApi.<T> cast(parseJson(res));
- type = JSON_TYPE;
+ JSONValue val = parseJson(res);
+ if (isJsonEncoded(res) && val.isString() != null) {
+ data = NativeString.wrap(val.isString().stringValue()).cast();
+ type = simpleType(res.getHeader("X-FYI-Content-Type"));
+ } else {
+ data = RestApi.<T> cast(val);
+ type = JSON_TYPE;
+ }
} catch (JSONException e) {
if (!background) {
RpcStatus.INSTANCE.onRpcComplete();
@@ -140,9 +146,6 @@
"Invalid JSON: " + e.getMessage()));
return;
}
- } else if (isEncodedBase64(res)) {
- data = NativeString.wrap(decodeBase64(res.getText())).cast();
- type = simpleType(res.getHeader("X-FYI-Content-Type"));
} else if (isTextBody(res)) {
data = NativeString.wrap(res.getText()).cast();
type = TEXT_TYPE;
@@ -371,7 +374,7 @@
public <T extends JavaScriptObject> void post(String content,
HttpCallback<T> cb) {
- sendRaw(POST, content, cb);
+ sendText(POST, content, cb);
}
public <T extends JavaScriptObject> void put(AsyncCallback<T> cb) {
@@ -389,7 +392,7 @@
public <T extends JavaScriptObject> void put(String content,
HttpCallback<T> cb) {
- sendRaw(PUT, content, cb);
+ sendText(PUT, content, cb);
}
public <T extends JavaScriptObject> void put(
@@ -423,10 +426,7 @@
private static native String str(JavaScriptObject jso)
/*-{ return JSON.stringify(jso) }-*/;
- private static native String decodeBase64(String a)
- /*-{ return $wnd.atob(a) }-*/;
-
- private <T extends JavaScriptObject> void sendRaw(Method method, String body,
+ private <T extends JavaScriptObject> void sendText(Method method, String body,
HttpCallback<T> cb) {
HttpImpl<T> httpCallback = new HttpImpl<>(background, cb);
try {
@@ -434,7 +434,7 @@
RpcStatus.INSTANCE.onRpcStart();
}
RequestBuilder req = request(method);
- req.setHeader("Content-Type", TEXT_TYPE);
+ req.setHeader("Content-Type", TEXT_UTF8);
req.sendRequest(body, httpCallback);
} catch (RequestException e) {
httpCallback.onError(null, e);
@@ -461,9 +461,8 @@
return isContentType(res, TEXT_TYPE);
}
- private static boolean isEncodedBase64(Response res) {
- return "base64".equals(res.getHeader("X-FYI-Content-Encoding"))
- && isTextBody(res);
+ private static boolean isJsonEncoded(Response res) {
+ return "json".equals(res.getHeader("X-FYI-Content-Encoding"));
}
private static boolean isContentType(Response res, String want) {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index a7b53cf..dd36495 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -91,6 +91,7 @@
import com.google.gson.JsonPrimitive;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
+import com.google.gson.stream.JsonWriter;
import com.google.gson.stream.MalformedJsonException;
import com.google.gwtexpui.server.CacheHeaders;
import com.google.inject.Inject;
@@ -719,6 +720,7 @@
}
}
+ @SuppressWarnings("resource")
static void replyBinaryResult(
@Nullable HttpServletRequest req,
HttpServletResponse res,
@@ -731,7 +733,11 @@
"attachment; filename=\"" + bin.getAttachmentName() + "\"");
}
if (bin.isBase64()) {
- bin = stackBase64(res, bin);
+ if (req != null && JSON_TYPE.equals(req.getHeader(HttpHeaders.ACCEPT))) {
+ bin = stackJsonString(res, bin);
+ } else {
+ bin = stackBase64(res, bin);
+ }
}
if (bin.canGzip() && acceptsGzip(req)) {
bin = stackGzip(res, bin);
@@ -758,6 +764,24 @@
}
}
+ private static BinaryResult stackJsonString(HttpServletResponse res,
+ final BinaryResult src) throws IOException {
+ TemporaryBuffer.Heap buf = heap(Integer.MAX_VALUE);
+ buf.write(JSON_MAGIC);
+ try(Writer w = new BufferedWriter(new OutputStreamWriter(buf, UTF_8));
+ JsonWriter json = new JsonWriter(w)) {
+ json.setLenient(true);
+ json.setHtmlSafe(true);
+ json.value(src.asString());
+ w.write('\n');
+ }
+ res.setHeader("X-FYI-Content-Encoding", "json");
+ res.setHeader("X-FYI-Content-Type", src.getContentType());
+ return asBinaryResult(buf)
+ .setContentType(JSON_TYPE)
+ .setCharacterEncoding(UTF_8.name());
+ }
+
private static BinaryResult stackBase64(HttpServletResponse res,
final BinaryResult src) throws IOException {
BinaryResult b64;
