✨ Handle header names in a case-insensitive manner when metadata is r…

…et… (#13887) * Handle header names in a case-insensitive manner when metadata is returned in the line-delimited JSON HTML format. * Fix lint errors. * Address review comments * Force the X-AmpAdRender header name to lowercase * Another lint error
ampproject · Mar 15, 2018 · 8782fb8 · 8782fb8
1 parent 84689b1
commit 8782fb8
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 10 deletions.
diff --git a/ads/google/a4a/line-delimited-response-handler.js b/ads/google/a4a/line-delimited-response-handler.js
@@ -72,10 +72,14 @@ export function metaJsonCreativeGrouper(callback) {
   let first;
   return function(line, done) {
     if (first) {
-      callback(
-          unescapeLineDelimitedHtml_(line),
-          /** @type {!Object<string, *>} */(tryParseJson(first) || {}),
-          done);
+      const metadata =
+          /** @type {!Object<string, *>} */(tryParseJson(first) || {});
+      const lowerCasedMetadata =
+          Object.keys(metadata).reduce((newObj, key) => {
+            newObj[key.toLowerCase()] = metadata[key];
+            return newObj;
+          }, {});
+      callback(unescapeLineDelimitedHtml_(line), lowerCasedMetadata, done);
       first = null;
     } else {
       first = line;

diff --git a/ads/google/a4a/test/test-line-delimited-response-handler.js b/ads/google/a4a/test/test-line-delimited-response-handler.js
@@ -66,12 +66,20 @@ describe('#line-delimited-response-handler', () => {
       // TODO: can't use objects as keys :(
       const calls = {};
       slotData.forEach(slot => {
-        const key = slot.creative + JSON.stringify(slot.headers);
+        const normalizedHeaderNames =
+            Object.keys(slot.headers).map(s => [s.toLowerCase(), s]);
+        slot.normalizedHeaders = {};
+        normalizedHeaderNames.forEach(
+            namePair =>
+              slot.normalizedHeaders[namePair[0]] = slot.headers[namePair[1]]);
+        const key = slot.creative + JSON.stringify(slot.normalizedHeaders);
         calls[key] ? calls[key]++ : (calls[key] = 1);
       });
       slotData.forEach(slot => {
-        expect(chunkHandlerStub.withArgs(slot.creative, slot.headers).callCount)
-            .to.equal(calls[slot.creative + JSON.stringify(slot.headers)]);
+        expect(chunkHandlerStub.withArgs(
+            slot.creative, slot.normalizedHeaders).callCount)
+            .to.equal(calls[slot.creative +
+                           JSON.stringify(slot.normalizedHeaders)]);
       });
     });
   }
@@ -186,7 +194,13 @@ describe('#line-delimited-response-handler', () => {
         {headers: {}, creative: ''},
         {headers: {foo: 'bar', hello: 'world'},
           creative: '\t\n\r<html>\bbaz\r</html>\n\n'},
+        {headers: {Foo: 'bar', hello: 'world'},
+          creative: '\t\n\r<html>\bbaz\r</html>\n\n'},
         {headers: {}, creative: ''},
+        {headers: {Foo: 'bar', HELLO: 'Le Monde'},
+          creative: '\t\n\r<html>\bbaz\r</html>\n\n'},
+        {headers: {FOO: 'bar', Hello: 'Le Monde'},
+          creative: '\t\n\r<html>\bbaz\r</html>\n\n'},
         {headers: {hello: 'world'},
           creative: '<html>\nchu\nnk me</h\rtml\n\t>'},
         {headers: {}, creative: ''},

diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
@@ -1373,14 +1373,15 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
                   (creative, headersObj, done) => {
                     checkStillCurrent();
                     // Force safeframe rendering method.
-                    headersObj[RENDERING_TYPE_HEADER] = XORIGIN_MODE.SAFEFRAME;
+                    headersObj[RENDERING_TYPE_HEADER.toLowerCase()] =
+                        XORIGIN_MODE.SAFEFRAME;
                     // Construct pseudo fetch response to be passed down the A4A
                     // promise chain for this block.
                     const headers =
                   /** @type {?../../../src/service/xhr-impl.FetchResponseHeaders} */
                   ({
-                    get: name => headersObj[name],
-                    has: name => !!headersObj[name],
+                    get: name => headersObj[name.toLowerCase()],
+                    has: name => !!headersObj[name.toLowerCase()],
                   });
                     const fetchResponse =
                   /** @type {?../../../src/service/xhr-impl.FetchResponse} */