Bug 1243594 (part 2) - have rest.js automatically encode the request body as utf-8. r=gfritzsche
authorMark Hammond <mhammond@skippinet.com.au>
Fri, 26 Feb 2016 15:46:30 +1100
changeset 323956 19950e70819a7f7a6089059cea59e04b9ef078fc
parent 323955 6decc397d3d5233fea362fccc072605594d7a689
child 323957 4e0bcdfc72de10bbfd2216a0d08081d9aecc3709
push id1128
push userjlund@mozilla.com
push dateWed, 01 Jun 2016 01:31:59 +0000
treeherdermozilla-release@fe0d30de989d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1243594
milestone47.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1243594 (part 2) - have rest.js automatically encode the request body as utf-8. r=gfritzsche
services/common/rest.js
services/common/tests/unit/test_restrequest.js
services/fxaccounts/tests/xpcshell/test_client.js
--- a/services/common/rest.js
+++ b/services/common/rest.js
@@ -317,33 +317,51 @@ RESTRequest.prototype = {
       } else {
         this._log.trace("HTTP Header " + key + ": " + headers[key]);
       }
       channel.setRequestHeader(key, headers[key], false);
     }
 
     // Set HTTP request body.
     if (method == "PUT" || method == "POST" || method == "PATCH") {
-      // Convert non-string bodies into JSON.
+      // Convert non-string bodies into JSON with utf-8 encoding. If a string
+      // is passed we assume they've already encoded it.
+      let contentType = headers["content-type"];
       if (typeof data != "string") {
         data = JSON.stringify(data);
+        if (!contentType) {
+          contentType = "application/json";
+        }
+        if (!contentType.includes("charset")) {
+          data = CommonUtils.encodeUTF8(data);
+          contentType += "; charset=utf-8";
+        } else {
+          // If someone handed us an object but also a custom content-type
+          // it's probably confused. We could go to even further lengths to
+          // respect it, but this shouldn't happen in practice.
+          Cu.reportError("rest.js found an object to JSON.stringify but also a " +
+                         "content-type header with a charset specification. " +
+                         "This probably isn't going to do what you expect");
+        }
+      }
+      if (!contentType) {
+        contentType = "text/plain";
       }
 
       this._log.debug(method + " Length: " + data.length);
       if (this._log.level <= Log.Level.Trace) {
         this._log.trace(method + " Body: " + data);
       }
 
       let stream = Cc["@mozilla.org/io/string-input-stream;1"]
                      .createInstance(Ci.nsIStringInputStream);
       stream.setData(data, data.length);
 
-      let type = headers["content-type"] || "text/plain";
       channel.QueryInterface(Ci.nsIUploadChannel);
-      channel.setUploadStream(stream, type, data.length);
+      channel.setUploadStream(stream, contentType, data.length);
     }
     // We must set this after setting the upload stream, otherwise it
     // will always be 'PUT'. Yeah, I know.
     channel.requestMethod = method;
 
     // Before opening the channel, set the charset that serves as a hint
     // as to what the response might be encoded as.
     channel.contentCharset = this.charset;
--- a/services/common/tests/unit/test_restrequest.js
+++ b/services/common/tests/unit/test_restrequest.js
@@ -216,16 +216,50 @@ add_test(function test_get_utf8() {
       do_check_eq(request2.response.charset, "utf-8");
 
       server.stop(run_next_test);
     });
   });
 });
 
 /**
+ * Test HTTP POST data is encoded as UTF-8 by default.
+ */
+add_test(function test_post_utf8() {
+  // We setup a handler that responds with exactly what it received.
+  // Given we've already tested above that responses are correctly utf-8
+  // decoded we can surmise that the correct response coming back means the
+  // input must also have been encoded.
+  let server = httpd_setup({"/echo": function(req, res) {
+    res.setStatusLine(req.httpVersion, 200, "OK");
+    res.setHeader("Content-Type", req.getHeader("content-type"));
+    // Get the body as bytes and write them back without touching them
+    let sis = Cc["@mozilla.org/scriptableinputstream;1"]
+              .createInstance(Ci.nsIScriptableInputStream);
+    sis.init(req.bodyInputStream);
+    let body = sis.read(sis.available());
+    sis.close()
+    res.write(body);
+  }});
+
+  let data = {copyright: "\xa9"}; // \xa9 is the copyright symbol
+  let request1 = new RESTRequest(server.baseURI + "/echo");
+  request1.post(data, function(error) {
+    do_check_null(error);
+
+    do_check_eq(request1.response.status, 200);
+    deepEqual(JSON.parse(request1.response.body), data);
+    do_check_eq(request1.response.headers["content-type"],
+                "application/json; charset=utf-8")
+
+    server.stop(run_next_test);
+  });
+});
+
+/**
  * Test more variations of charset handling.
  */
 add_test(function test_charsets() {
   let response = "Hello World, I can't speak Russian";
 
   let contentType = "text/plain";
   let charset = true;
   let charsetSuffix = "; charset=us-ascii";
@@ -426,17 +460,17 @@ add_test(function test_put_json() {
 
     do_check_eq(this.status, this.COMPLETED);
     do_check_true(this.response.success);
     do_check_eq(this.response.status, 200);
     do_check_eq(this.response.body, "");
 
     do_check_eq(handler.request.method, "PUT");
     do_check_eq(handler.request.body, JSON.stringify(sample_data));
-    do_check_eq(handler.request.getHeader("Content-Type"), "text/plain");
+    do_check_eq(handler.request.getHeader("Content-Type"), "application/json; charset=utf-8");
 
     server.stop(run_next_test);
   });
 });
 
 /**
  * The 'data' argument to POST, if not a string already, is automatically
  * stringified as JSON.
@@ -456,16 +490,42 @@ add_test(function test_post_json() {
 
     do_check_eq(this.status, this.COMPLETED);
     do_check_true(this.response.success);
     do_check_eq(this.response.status, 200);
     do_check_eq(this.response.body, "");
 
     do_check_eq(handler.request.method, "POST");
     do_check_eq(handler.request.body, JSON.stringify(sample_data));
+    do_check_eq(handler.request.getHeader("Content-Type"), "application/json; charset=utf-8");
+
+    server.stop(run_next_test);
+  });
+});
+
+/**
+ * The content-type will be text/plain without a charset if the 'data' argument
+ * to POST is already a string.
+ */
+add_test(function test_post_json() {
+  let handler = httpd_handler(200, "OK");
+  let server = httpd_setup({"/resource": handler});
+
+  let sample_data = "hello";
+  let request = new RESTRequest(server.baseURI + "/resource");
+  request.post(sample_data, function (error) {
+    do_check_eq(error, null);
+
+    do_check_eq(this.status, this.COMPLETED);
+    do_check_true(this.response.success);
+    do_check_eq(this.response.status, 200);
+    do_check_eq(this.response.body, "");
+
+    do_check_eq(handler.request.method, "POST");
+    do_check_eq(handler.request.body, sample_data);
     do_check_eq(handler.request.getHeader("Content-Type"), "text/plain");
 
     server.stop(run_next_test);
   });
 });
 
 /**
  * HTTP PUT with a custom Content-Type header.
--- a/services/fxaccounts/tests/xpcshell/test_client.js
+++ b/services/fxaccounts/tests/xpcshell/test_client.js
@@ -168,31 +168,35 @@ add_task(function* test_signUp() {
   let creationMessage_withKey = JSON.stringify({
     uid: "uid",
     sessionToken: "sessionToken",
     keyFetchToken: "keyFetchToken"
   });
   let errorMessage = JSON.stringify({code: 400, errno: 101, error: "account exists"});
   let created = false;
 
+  // Note these strings must be unicode and not already utf-8 encoded.
+  let unicodeUsername = "andr\xe9@example.org"; // 'andré@example.org'
+  let unicodePassword = "p\xe4ssw\xf6rd"; // 'pässwörd'
   let server = httpd_setup({
     "/account/create": function(request, response) {
       let body = CommonUtils.readBytesFromInputStream(request.bodyInputStream);
+      body = CommonUtils.decodeUTF8(body);
       let jsonBody = JSON.parse(body);
 
       // https://github.com/mozilla/fxa-auth-server/wiki/onepw-protocol#wiki-test-vectors
 
       if (created) {
         // Error trying to create same account a second time
         response.setStatusLine(request.httpVersion, 400, "Bad request");
         response.bodyOutputStream.write(errorMessage, errorMessage.length);
         return;
       }
 
-      if (jsonBody.email == "andré@example.org") {
+      if (jsonBody.email == unicodeUsername) {
         do_check_eq("", request._queryString);
         do_check_eq(jsonBody.authPW, "247b675ffb4c46310bc87e26d712153abe5e1c90ef00a4784594f97ef54f2375");
 
         response.setStatusLine(request.httpVersion, 200, "OK");
         response.bodyOutputStream.write(creationMessage_noKey,
                                         creationMessage_noKey.length);
         return;
       }
@@ -202,39 +206,42 @@ add_task(function* test_signUp() {
         do_check_eq(jsonBody.authPW, "e5c1cdfdaa5fcee06142db865b212cc8ba8abee2a27d639d42c139f006cdb930");
         created = true;
 
         response.setStatusLine(request.httpVersion, 200, "OK");
         response.bodyOutputStream.write(creationMessage_withKey,
                                         creationMessage_withKey.length);
         return;
       }
+      // just throwing here doesn't make any log noise, so have an assertion
+      // fail instead.
+      do_check_true(false, "unexpected email: " + jsonBody.email);
     },
   });
 
   // Try to create an account without retrieving optional keys.
   let client = new FxAccountsClient(server.baseURI);
-  let result = yield client.signUp('andré@example.org', 'pässwörd');
+  let result = yield client.signUp(unicodeUsername, unicodePassword);
   do_check_eq("uid", result.uid);
   do_check_eq("sessionToken", result.sessionToken);
   do_check_eq(undefined, result.keyFetchToken);
   do_check_eq(result.unwrapBKey,
               "de6a2648b78284fcb9ffa81ba95803309cfba7af583c01a8a1a63e567234dd28");
 
   // Try to create an account retrieving optional keys.
   result = yield client.signUp('you@example.org', 'pässwörd', true);
   do_check_eq("uid", result.uid);
   do_check_eq("sessionToken", result.sessionToken);
   do_check_eq("keyFetchToken", result.keyFetchToken);
   do_check_eq(result.unwrapBKey,
               "f589225b609e56075d76eb74f771ff9ab18a4dc0e901e131ba8f984c7fb0ca8c");
 
   // Try to create an existing account.  Triggers error path.
   try {
-    result = yield client.signUp('andré@example.org', 'pässwörd');
+    result = yield client.signUp(unicodeUsername, unicodePassword);
     do_throw("Expected to catch an exception");
   } catch(expectedError) {
     do_check_eq(101, expectedError.errno);
   }
 
   yield deferredStop(server);
 });
 
@@ -253,22 +260,25 @@ add_task(function* test_signIn() {
   });
   let errorMessage_wrongCap = JSON.stringify({
     code: 400,
     errno: 120,
     error: "Incorrect email case",
     email: "you@example.com"
   });
 
+  // Note this strings must be unicode and not already utf-8 encoded.
+  let unicodeUsername = "m\xe9@example.com" // 'mé@example.com'
   let server = httpd_setup({
     "/account/login": function(request, response) {
       let body = CommonUtils.readBytesFromInputStream(request.bodyInputStream);
+      body = CommonUtils.decodeUTF8(body);
       let jsonBody = JSON.parse(body);
 
-      if (jsonBody.email == "mé@example.com") {
+      if (jsonBody.email == unicodeUsername) {
         do_check_eq("", request._queryString);
         do_check_eq(jsonBody.authPW, "08b9d111196b8408e8ed92439da49206c8ecfbf343df0ae1ecefcd1e0174a8b6");
         response.setStatusLine(request.httpVersion, 200, "OK");
         response.bodyOutputStream.write(sessionMessage_noKey,
                                         sessionMessage_noKey.length);
         return;
       }
       else if (jsonBody.email == "you@example.com") {
@@ -293,17 +303,17 @@ add_task(function* test_signIn() {
                                         errorMessage_notExistent.length);
         return;
       }
     },
   });
 
   // Login without retrieving optional keys
   let client = new FxAccountsClient(server.baseURI);
-  let result = yield client.signIn('mé@example.com', 'bigsecret');
+  let result = yield client.signIn(unicodeUsername, 'bigsecret');
   do_check_eq(FAKE_SESSION_TOKEN, result.sessionToken);
   do_check_eq(result.unwrapBKey,
               "c076ec3f4af123a615157154c6e1d0d6293e514fd7b0221e32d50517ecf002b8");
   do_check_eq(undefined, result.keyFetchToken);
 
   // Login with retrieving optional keys
   result = yield client.signIn('you@example.com', 'bigsecret', true);
   do_check_eq(FAKE_SESSION_TOKEN, result.sessionToken);