Skip to content

Make response headers available to the specs#1748

Merged
mkendall07 merged 5 commits intomasterfrom
forward-response-headers
Oct 24, 2017
Merged

Make response headers available to the specs#1748
mkendall07 merged 5 commits intomasterfrom
forward-response-headers

Conversation

@dbemiller
Copy link
Contributor

Type of change

  • Feature

Description of change

Some adapters need to pull information from their server's response headers.

This adds that feature, and updates existing adapters so they continue to work.

Other information

Fixes #1742.


function headerParser(xmlHttpResponse) {
return {
get: responseObj.getResponseHeader.bind(responseObj)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do a bind here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paranoia... because I've been burned by this bugs too many times.

The root of the issue is: I don't know how browsers implement XMLHttpRequest.getResponseHeader.

Suppose I just did:

return {
  get: responseObj.getResponseHeader
}

but then they implemented it as something like this:

XmlHttpRequest.prototype.getResponseHeader = function(header) {
  var headers = this.parseResponseHeaders();
  return headers[header];
}

When an adapter calls get, then this evaluates to our object--not the original XmlHttpRequest. Since we don't define parseResponseHeaders, it'd crash on an undefined function error.

I have no idea if this is a real risk... but the tl;dr is that this sucks. Since I can't eject it from the language, defensive binds keep the code safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok cool

@mkendall07 mkendall07 merged commit 88af47b into master Oct 24, 2017
@mkendall07 mkendall07 deleted the forward-response-headers branch October 24, 2017 19:16
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Oct 25, 2017
* 'master' of https://github.com/prebid/Prebid.js: (414 commits)
  Make response headers available to the specs (prebid#1748)
  add option to run tests in a specific file (prebid#1727)
  Update JCM Adapter to 1.0  (prebid#1715)
  Finished an unfinished comment. (prebid#1749)
  Platform.io Bidder Adapter update.  Prebid v1.0. (prebid#1705)
  Fix window.top.host cross origin issue when in nested iframes. (prebid#1730)
  fix log message not displaying when referencing missing bidder (prebid#1737)
  Allow more than one placement from one page (prebid#1692)
  Justpremium Adapter bugfix (prebid#1716)
  Updating license (prebid#1717)
  realvuBidAdapter  (prebid#1571)
  Update JSDoc to call the module `pbjs` (prebid#1572)
  Update Beachfront adapter for v1.0 (prebid#1675)
  Update AdButler adapter for Prebid v1.0 (prebid#1664)
  Increment pre version
  Fix for prebid#1628 (allowing standard bidCpmAdjustment) (prebid#1645)
  Prebid 0.31.0 Release
  Support native click tracking (prebid#1691)
  Initial commit for video support for pbs (prebid#1706)
  Fixes: Immediate adapter response may end auction (prebid#1690)
  ...
@matthewlane matthewlane mentioned this pull request Nov 1, 2017
8 tasks
@jaiminpanchal27 jaiminpanchal27 mentioned this pull request Nov 3, 2017
1 task
@matthewlane matthewlane mentioned this pull request Nov 8, 2017
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments