From d0d3602808faf6998da1abc01192ee4984344291 Mon Sep 17 00:00:00 2001
From: lutangar <johan.dufour@gmail.com>
Date: Fri, 12 May 2017 15:29:19 +0200
Subject: [PATCH] allow valid accept headers in document loader options

---
 js/jsonld.js                       |  50 +++++++++-----
 test/buildHeaders.test.js          | 103 +++++++++++++++++++++++++++++
 test/node-document-loader-tests.js |   4 +-
 3 files changed, 140 insertions(+), 17 deletions(-)
 create mode 100644 test/buildHeaders.test.js

diff --git a/js/jsonld.js b/js/jsonld.js
index ce40764a..9d1bacfe 100644
--- a/js/jsonld.js
+++ b/js/jsonld.js
@@ -1650,35 +1650,55 @@ jsonld.cache = {
  */
 var _defaults = {
   headers: {
-    accept: 'application/ld+json, application/json'
+    accept: ['application/ld+json', 'application/json']
   }
 };
 
 /**
- * Build an headers object from custom headers and assert `accept` header isn't overridden.
+ * Build an headers object from custom headers and assert `accept` header is valid.
  *
  * @param {Object} optionsHeaders an object (map) of headers
  *          with key as header name and value as header value.
  * @return {Object} an object (map) of headers with a valid `accept` header.
  */
-function buildHeaders(optionsHeaders) {
+jsonld.buildHeaders = function(optionsHeaders) {
   optionsHeaders = optionsHeaders || {};
 
-  var hasAccept = Object.keys(optionsHeaders).some(function(h) {
+  var headers = {};
+  var acceptHeaderKeys = Object.keys(optionsHeaders).filter(function(h) {
     return h.toLowerCase() === 'accept';
   });
 
-  if(hasAccept) {
-    throw new RangeError(
-      'Accept header may not be specified as an option; only "' +
-      _defaults.headers.accept + '" is supported.');
+  if(acceptHeaderKeys.length === 1) { // only one `accept` header key may be ok
+    // check if `accept` header is valid
+    var acceptHeaderValues = optionsHeaders[acceptHeaderKeys[0]]
+      .split(',')
+      .map(function(value) {
+        return value.trim().toLowerCase();
+      });
+
+    var acceptHeaderIsValid = _defaults.headers.accept.every(function(requiredAcceptHeaderValue) {
+      return acceptHeaderValues.indexOf(requiredAcceptHeaderValue) !== -1;
+    });
+
+    if(!acceptHeaderIsValid) {
+      throw new RangeError('Accept header must contains "' +
+        _defaults.headers.accept.join(', ') + '".');
+    }
+
+    headers['Accept'] = acceptHeaderValues.join(', ');
+  } else if(acceptHeaderKeys.length >= 2) { // there are too many accept headers
+    throw new RangeError('Accept header may be specified only once.');
+  } else { // no accept headers take the default values
+    headers['Accept'] = _defaults.headers.accept.join(', ');
   }
 
-  var headers = {'Accept': _defaults.headers.accept};
-  for(var k in optionsHeaders) { headers[k] = optionsHeaders[k]; }
+  for(var k in optionsHeaders) {
+    if(k.toLowerCase() !== 'accept') { headers[k] = optionsHeaders[k]; }
+  }
 
   return headers;
-}
+};
 
 /**
  * Document loaders.
@@ -1702,7 +1722,7 @@ jsonld.documentLoaders = {};
 jsonld.documentLoaders.jquery = function($, options) {
   options = options || {};
   var queue = new jsonld.RequestQueue();
-  var headers = buildHeaders(options.headers);
+  var headers = jsonld.buildHeaders(options.headers);
 
   // use option or, by default, use Promise when its defined
   var usePromise = ('usePromise' in options ?
@@ -1732,7 +1752,7 @@ jsonld.documentLoaders.jquery = function($, options) {
     $.ajax({
       url: url,
       accepts: {
-        json: _defaults.headers.accept
+        json: _defaults.headers.accept.join(', ')
       },
       headers: headers,
       dataType: 'json',
@@ -1791,7 +1811,7 @@ jsonld.documentLoaders.jquery = function($, options) {
  */
 jsonld.documentLoaders.node = function(options) {
   options = options || {};
-  var headers = buildHeaders(options.headers);
+  var headers = jsonld.buildHeaders(options.headers);
   var strictSSL = ('strictSSL' in options) ? options.strictSSL : true;
   var maxRedirects = ('maxRedirects' in options) ? options.maxRedirects : -1;
   var request = ('request' in options) ? options.request : require('request');
@@ -1934,7 +1954,7 @@ jsonld.documentLoaders.xhr = function(options) {
   options = options || {};
   var rlink = /(^|(\r\n))link:/i;
   var queue = new jsonld.RequestQueue();
-  var headers = buildHeaders(options.headers);
+  var headers = jsonld.buildHeaders(options.headers);
 
   // use option or, by default, use Promise when its defined
   var usePromise = ('usePromise' in options ?
diff --git a/test/buildHeaders.test.js b/test/buildHeaders.test.js
new file mode 100644
index 00000000..23792806
--- /dev/null
+++ b/test/buildHeaders.test.js
@@ -0,0 +1,103 @@
+var jsonld = require('../js/jsonld');
+var assert = require('assert');
+
+describe('Build a valid header object from pre-defined headers', function() {
+
+  describe('When built with 2 accept headers', function() {
+    var headers = {
+      'Accept': 'application/json',
+      'accept': 'application/ld+json, application/json'
+    };
+
+
+    it('should fail', function() {
+      assert.throws(
+        jsonld.buildHeaders.bind(null, headers),
+        function(err) {
+          assert.ok(err instanceof RangeError, 'A range error should be thrown');
+          assert.equal(err.message, 'Accept header may be specified only once.');
+
+          return true;
+        }
+      );
+    });
+  });
+
+  describe('When built with no explicit headers', function() {
+    var headers = {};
+    it(
+      'the "Accept" header should default to "application/ld+json, application/json"',
+      function() {
+        var actualHeaders = jsonld.buildHeaders(headers);
+        assert.deepEqual(actualHeaders, {
+          'Accept': 'application/ld+json, application/json'
+        });
+      }
+    );
+  });
+
+  describe('When built with custom headers', function() {
+    var headers = {
+      'Authorization': 'Bearer d783jkjaods9f87o83hj'
+    };
+    it('the custom headers should be preserved', function() {
+      var actualHeaders = jsonld.buildHeaders(headers);
+      assert.deepEqual(actualHeaders, {
+        'Authorization': 'Bearer d783jkjaods9f87o83hj',
+        'Accept': 'application/ld+json, application/json'
+      });
+    });
+  });
+
+  describe('When built with an accept header equal to the default accept header value', function() {
+    var headers = {
+      'Accept': 'application/ld+json, application/json'
+    };
+    it(
+      'the accept header should remain unchanged',
+      function() {
+        var actualHeaders = jsonld.buildHeaders(headers);
+        assert.deepEqual(actualHeaders, headers);
+      }
+    );
+  });
+
+  describe('When built with a valid accept headers with extra acceptations', function() {
+    var headers = {
+      'Accept': 'application/ld+json, application/json, text/html'
+    };
+    it(
+      'the accept header should remain unchanged',
+      function() {
+        var actualHeaders = jsonld.buildHeaders(headers);
+        assert.deepEqual(actualHeaders, headers);
+      }
+    );
+  });
+
+  describe('When built with an invalid accept header', function() {
+    var possibleInvalidHeaders = [
+      { 'Accept': 'text/html' },
+      { 'Accept': 'application/ld+json, application/jsonbourne' },
+      { 'Accept': 'application/ld+json application/json' }
+    ];
+
+    for (var i in possibleInvalidHeaders) {
+      console.log(possibleInvalidHeaders[i]);
+      it('should fail', function() {
+        assert.throws(
+          jsonld.buildHeaders.bind(null, possibleInvalidHeaders[i]),
+          function(err) {
+            assert.ok(err instanceof RangeError, 'A range error should be thrown');
+            assert.equal(
+              err.message,
+              'Accept header must contains "application/ld+json, application/json".'
+            );
+
+            return true;
+          }
+        );
+      });
+    }
+  });
+});
diff --git a/test/node-document-loader-tests.js b/test/node-document-loader-tests.js
index fb522012..f94a8bc8 100644
--- a/test/node-document-loader-tests.js
+++ b/test/node-document-loader-tests.js
@@ -78,7 +78,7 @@ describe('For the node.js document loader', function() {
     });
   });
 
-  describe('When built using headers that already contain an Accept header', function() {
+  describe('When built using headers that already contain an invalid Accept header', function() {
     var options = {request: requestMock};
     options.headers = {
       'x-test-header-3': 'Third value',
@@ -86,7 +86,7 @@ describe('For the node.js document loader', function() {
     };
 
     it('constructing the document loader should fail', function() {
-      var expectedMessage = 'Accept header may not be specified as an option; only "application/ld+json, application/json" is supported.';
+      var expectedMessage = 'Accept header must contains "application/ld+json, application/json".';
       assert.throws(
         jsonld.useDocumentLoader.bind(jsonld, documentLoaderType, options),
         function(err) {