-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow bypassing ajax preflights with config options #630
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @matthewlane
can you update per my comments? Thanks
@@ -54,8 +54,11 @@ export function ajax(url, callback, data, options = {}) { | |||
if (options.withCredentials) { | |||
x.withCredentials = true; | |||
} else { | |||
x.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); | |||
x.setRequestHeader('Content-Type', 'application/json;charset=UTF-8'); | |||
if (options.preflight !== false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for the double negation here? Also don't see why we can't use withCredentials
with preflight
. IMO these should all be standalone options
x.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); | ||
} | ||
x.setRequestHeader('Content-Type', | ||
options.contentType || 'application/json;charset=UTF-8'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will also have to be updated, as content other than application/x-www-form-urlencoded, multipart/form-data, or text/plain
requires a preflight request. In other words, application/json
will trigger preflight.
@@ -54,8 +54,11 @@ export function ajax(url, callback, data, options = {}) { | |||
if (options.withCredentials) { | |||
x.withCredentials = true; | |||
} else { | |||
x.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); | |||
x.setRequestHeader('Content-Type', 'application/json;charset=UTF-8'); | |||
if (options.preflight !== false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These code will let preflight requests become default behavior. Is it intential?!
LGTM. |
…3.0 to master * commit '9d4bc7fda6bcc249544fb8636b98e90fdc8d474b': (44 commits) Added support for new adapters introduced in 0.13.0 into AOL analytics Updated CHANGELOG Fixed merge conflict Prebid 0.13.0 Release Resolves prebid#635 (prebid#640) Add IX Deal Support (prebid#638) Bug fix: accept custom timeout prebid#582 & prebid#604 (prebid#641) added timeouts to rubicon adapter that consider time-to-start costs (prebid#629) Modify handling of no-bids in Krux Link adapter (prebid#628) Allow bypassing ajax preflights with config options (prebid#630) validateIndentation: Invalid indentation character (prebid#631) use `splice` rather than `slice` to remove bids from array (prebid#637) delete the callback before calling clearAuction (prebid#636) Deal override fix to resolve prebid#618 (prebid#619) Update package.json Video - create mastertagUrl from adservertag (prebid#614) Prevent renderAd from rendering videos (prebid#623) Remove unused build depencencies (prebid#622) Add pull request template (prebid#615) Openx adaptor deal update (prebid#612) ...
* Allow bypassing ajax preflights with config options * Make preflight opt-in and set default content-type to text-plain
* Allow bypassing ajax preflights with config options * Make preflight opt-in and set default content-type to text-plain
Type of change
Description of change
This change allows ajax calls to be configured with two options related to sending preflight OPTIONS requests. Setting
preflight
to false, and settingcontentType
to one ofapplication/x-www-form-urlencoded
,multipart/form-data
, ortext/plain
will prevent a preflight OPTIONS request. Example of setting these options:Other information
Addresses #625