Bug 1243594 (part 2) - have rest.js automatically encode the request body as utf-8. r=gfritzsche
☠☠ backed out by 97f2c31df5f5 ☠ ☠
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 16 Feb 2016 12:44:49 +1100
changeset 331222 5688da9024b661ebd6db4ef81bd19c59b4d15386
parent 331221 4f941cddcf34462a4b15694000bf3388e1b00cc4
child 331223 97f2c31df5f5b5c36992a8167e3cdc26fb8748d9
push id10938
push userBogdan.Postelnicu@softvision.ro
push dateTue, 16 Feb 2016 15:52:05 +0000
reviewersgfritzsche
bugs1243594
milestone47.0a1
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);