Bug 694735 - Change default Accept header to json in Sync API requests r?eoger draft
authorCarol Ng <cng@mozilla.com>
Thu, 24 May 2018 15:04:11 -0400
changeset 799481 8c24022ecbf6c796b03b8f5e4a40f4748569e13b
parent 798691 d36cd8bdbc5c0df1d1d7a167f5fedb95c3a3648e
push id111079
push userbmo:cng@mozilla.com
push dateThu, 24 May 2018 19:52:37 +0000
reviewerseoger
bugs694735
milestone62.0a1
Bug 694735 - Change default Accept header to json in Sync API requests r?eoger
services/common/rest.js
services/common/tests/unit/test_restrequest.js
services/sync/modules/resource.js
services/sync/tests/unit/test_resource.js
--- a/services/common/rest.js
+++ b/services/common/rest.js
@@ -270,16 +270,21 @@ RESTRequest.prototype = {
       if (key == "authorization") {
         this._log.trace("HTTP Header " + key + ": ***** (suppressed)");
       } else {
         this._log.trace("HTTP Header " + key + ": " + headers[key]);
       }
       channel.setRequestHeader(key, headers[key], false);
     }
 
+    // REST requests accept JSON by default
+    if (!headers.accept) {
+      channel.setRequestHeader("accept", "application/json;q=0.9,*/*;q=0.2", false);
+    }
+
     // Set HTTP request body.
     if (method == "PUT" || method == "POST" || method == "PATCH") {
       // 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) {
--- a/services/common/tests/unit/test_restrequest.js
+++ b/services/common/tests/unit/test_restrequest.js
@@ -544,16 +544,41 @@ add_task(async function test_get_no_head
       do_throw("Got unexpected header!");
     }
   }
 
   await promiseStopServer(server);
 });
 
 /**
+ * Client includes default Accept header in API requests
+ */
+add_task(async function test_default_accept_headers() {
+  let handler = httpd_handler(200, "OK");
+  let server = httpd_setup({"/resource": handler});
+
+  let request = new RESTRequest(server.baseURI + "/resource");
+  await request.get();
+
+  Assert.equal(request.response.status, 200);
+  Assert.equal(request.response.body, "");
+
+  let accept_header = handler.request.getHeader("accept");
+
+  Assert.ok(!accept_header.includes("text/html"));
+  Assert.ok(!accept_header.includes("application/xhtml+xml"));
+  Assert.ok(!accept_header.includes("applcation/xml"));
+
+  Assert.ok(accept_header.includes("application/json") ||
+            accept_header.includes("application/newlines"));
+
+  await promiseStopServer(server);
+});
+
+/**
  * Test changing the URI after having created the request.
  */
 add_task(async function test_changing_uri() {
   let handler = httpd_handler(200, "OK");
   let server = httpd_setup({"/resource": handler});
 
   let request = new RESTRequest("http://localhost:1234/the-wrong-resource");
   request.uri = CommonUtils.makeURI(server.baseURI + "/resource");
--- a/services/sync/modules/resource.js
+++ b/services/sync/modules/resource.js
@@ -120,16 +120,20 @@ Resource.prototype = {
         if (k == "authorization") {
           this._log.trace(`HTTP Header ${k}: ***** (suppressed)`);
         } else {
           this._log.trace(`HTTP Header ${k}: ${v}`);
         }
       }
     }
 
+    if (!headers.has("accept")) {
+      headers.append("accept", "application/json;q=0.9,*/*;q=0.2");
+    }
+
     return headers;
   },
 
   /**
    * @param {string} method HTTP method
    * @param {string} data HTTP body
    * @param {object} signal AbortSignal instance
    * @returns {Request}
--- a/services/sync/tests/unit/test_resource.js
+++ b/services/sync/tests/unit/test_resource.js
@@ -121,17 +121,17 @@ function server_quota_notice(request, re
 function server_quota_error(request, response) {
   let body = "14";
   response.setHeader("X-Weave-Quota-Remaining", "-1024", false);
   response.setStatusLine(request.httpVersion, 400, "OK");
   response.bodyOutputStream.write(body, body.length);
 }
 
 function server_headers(metadata, response) {
-  let ignore_headers = ["host", "user-agent", "accept", "accept-language",
+  let ignore_headers = ["host", "user-agent", "accept-language",
                         "accept-encoding", "accept-charset", "keep-alive",
                         "connection", "pragma", "origin", "cache-control",
                         "content-length"];
   let headers = metadata.headers;
   let header_names = [];
   while (headers.hasMoreElements()) {
     let header = headers.getNext().toString();
     if (!ignore_headers.includes(header)) {
@@ -386,73 +386,75 @@ add_task(async function test_weave_times
   // Before having received any response containing the
   // X-Weave-Timestamp header, Resource.serverTime is null.
   Assert.equal(Resource.serverTime, null);
   let res8 = new Resource(server.baseURI + "/timestamp");
   await res8.get();
   Assert.equal(Resource.serverTime, TIMESTAMP);
 });
 
-add_task(async function test_get_no_headers() {
-  _("GET: no special request headers");
+add_task(async function test_get_default_headers() {
+  _("GET: Accept defaults to application/json");
   let res_headers = new Resource(server.baseURI + "/headers");
-  let content = await res_headers.get();
-  Assert.equal(content.data, "{}");
+  let content = JSON.parse((await res_headers.get()).data);
+  Assert.equal(content.accept, "application/json;q=0.9,*/*;q=0.2");
 });
 
-add_task(async function test_put_default_content_type() {
-  _("PUT: Content-Type defaults to text/plain");
+add_task(async function test_put_default_headers() {
+  _("PUT: Accept defaults to application/json, Content-Type defaults to text/plain");
   let res_headers = new Resource(server.baseURI + "/headers");
-  let content = await res_headers.put("data");
-  Assert.equal(content.data, JSON.stringify({"content-type": "text/plain"}));
+  let content = JSON.parse((await res_headers.put("data")).data);
+  Assert.equal(content.accept, "application/json;q=0.9,*/*;q=0.2");
+  Assert.equal(content["content-type"], "text/plain");
 });
 
-add_task(async function test_post_default_content_type() {
-  _("POST: Content-Type defaults to text/plain");
+add_task(async function test_post_default_headers() {
+  _("POST: Accept defaults to application/json, Content-Type defaults to text/plain");
   let res_headers = new Resource(server.baseURI + "/headers");
-  let content = await res_headers.post("data");
-  Assert.equal(content.data, JSON.stringify({"content-type": "text/plain"}));
+  let content = JSON.parse((await res_headers.post("data")).data);
+  Assert.equal(content.accept, "application/json;q=0.9,*/*;q=0.2");
+  Assert.equal(content["content-type"], "text/plain");
 });
 
 add_task(async function test_setHeader() {
   _("setHeader(): setting simple header");
   let res_headers = new Resource(server.baseURI + "/headers");
   res_headers.setHeader("X-What-Is-Weave", "awesome");
   Assert.equal(res_headers.headers["x-what-is-weave"], "awesome");
-  let content = await res_headers.get();
-  Assert.equal(content.data, JSON.stringify({"x-what-is-weave": "awesome"}));
+  let content = JSON.parse((await res_headers.get()).data);
+  Assert.equal(content["x-what-is-weave"], "awesome");
 });
 
 add_task(async function test_setHeader_overwrite() {
   _("setHeader(): setting multiple headers, overwriting existing header");
   let res_headers = new Resource(server.baseURI + "/headers");
   res_headers.setHeader("X-WHAT-is-Weave", "more awesomer");
   res_headers.setHeader("X-Another-Header", "hello world");
   Assert.equal(res_headers.headers["x-what-is-weave"], "more awesomer");
   Assert.equal(res_headers.headers["x-another-header"], "hello world");
-  let content = await res_headers.get();
-  Assert.equal(content.data, JSON.stringify({"x-another-header": "hello world",
-                                             "x-what-is-weave": "more awesomer"}));
+  let content = JSON.parse((await res_headers.get()).data);
+  Assert.equal(content["x-what-is-weave"], "more awesomer");
+  Assert.equal(content["x-another-header"], "hello world");
 });
 
 add_task(async function test_put_override_content_type() {
   _("PUT: override default Content-Type");
   let res_headers = new Resource(server.baseURI + "/headers");
   res_headers.setHeader("Content-Type", "application/foobar");
   Assert.equal(res_headers.headers["content-type"], "application/foobar");
-  let content = await res_headers.put("data");
-  Assert.equal(content.data, JSON.stringify({"content-type": "application/foobar"}));
+  let content = JSON.parse((await res_headers.put("data")).data);
+  Assert.equal(content["content-type"], "application/foobar");
 });
 
 add_task(async function test_post_override_content_type() {
   _("POST: override default Content-Type");
   let res_headers = new Resource(server.baseURI + "/headers");
   res_headers.setHeader("Content-Type", "application/foobar");
-  let content = await res_headers.post("data");
-  Assert.equal(content.data, JSON.stringify({"content-type": "application/foobar"}));
+  let content = JSON.parse((await res_headers.post("data")).data);
+  Assert.equal(content["content-type"], "application/foobar");
 });
 
 add_task(async function test_weave_backoff() {
   _("X-Weave-Backoff header notifies observer");
   let backoffInterval;
   function onBackoff(subject, data) {
     backoffInterval = subject;
   }