From d2c1fd4287a2f32cf86eb1d6f47f0bc3870838e3 Mon Sep 17 00:00:00 2001 From: Jason Woods Date: Mon, 29 Nov 2021 14:34:45 +0000 Subject: [PATCH] fix: when crossDomain is using default the X-Requested-With header is no longer added incorrectly When this header is added incorrectly, it triggers preflight requests. This caused a breaking change from rxjs 6.x with any backend request URL that does not support preflight requests (OPTIONS method). Issue traces back to https://github.com/ReactiveX/rxjs/commit/0d66ba458f07fba51cfc73440d01ef453c24cda7 --- spec/observables/dom/ajax-spec.ts | 47 +++++++++++++++++++------------ src/internal/ajax/ajax.ts | 29 +++++++++++-------- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/spec/observables/dom/ajax-spec.ts b/spec/observables/dom/ajax-spec.ts index a3457f2d9a..2402036359 100644 --- a/spec/observables/dom/ajax-spec.ts +++ b/spec/observables/dom/ajax-spec.ts @@ -103,7 +103,6 @@ describe('ajax', () => { 'content-type': 'kenny/loggins', 'fly-into-the': 'Dangah Zone!', 'take-a-ride-into-the': 'Danger ZoooOoone!', - 'x-requested-with': 'XMLHttpRequest', }); // Did not mutate the headers passed expect(obj.headers).to.deep.equal({ @@ -129,6 +128,7 @@ describe('ajax', () => { url: '/some/path', xsrfCookieName: 'foo', xsrfHeaderName: 'Custom-Header-Name', + crossDomain: false, }; ajax(obj).subscribe(); @@ -188,12 +188,33 @@ describe('ajax', () => { const request = MockXMLHttpRequest.mostRecent; expect(request.url).to.equal('/some/page'); - expect(request.requestHeaders).to.deep.equal({ - 'x-requested-with': 'XMLHttpRequest', - }); + expect(request.requestHeaders).to.deep.equal({}); }); }); + it('should not set the X-Requested-With if crossDomain is true', () => { + ajax({ + url: '/test/monkey', + method: 'GET', + crossDomain: true, + }).subscribe(); + + const request = MockXMLHttpRequest.mostRecent; + + expect(request.requestHeaders).to.not.have.key('x-requested-with'); + }); + + it('should not set the X-Requested-With if crossDomain is not present (it defaults to true)', () => { + ajax({ + url: '/test/monkey', + method: 'GET', + }).subscribe(); + + const request = MockXMLHttpRequest.mostRecent; + + expect(request.requestHeaders).to.not.have.key('x-requested-with'); + }); + it('should set the X-Requested-With if crossDomain is false', () => { ajax({ url: '/test/monkey', @@ -590,9 +611,7 @@ describe('ajax', () => { expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet'); expect(MockXMLHttpRequest.mostRecent.data).to.equal(body); - expect(MockXMLHttpRequest.mostRecent.requestHeaders).to.deep.equal({ - 'x-requested-with': 'XMLHttpRequest', - }); + expect(MockXMLHttpRequest.mostRecent.requestHeaders).to.deep.equal({}); }); it('should send the URLSearchParams straight through to the body', () => { @@ -789,7 +808,6 @@ describe('ajax', () => { expect(request.url).to.equal('/flibbertyJibbet'); expect(request.requestHeaders).to.deep.equal({ 'content-type': 'application/json;charset=utf-8', - 'x-requested-with': 'XMLHttpRequest', }); request.respondWith({ @@ -820,9 +838,7 @@ describe('ajax', () => { expect(request.method).to.equal('POST'); expect(request.url).to.equal('/flibbertyJibbet'); - expect(request.requestHeaders).to.deep.equal({ - 'x-requested-with': 'XMLHttpRequest', - }); + expect(request.requestHeaders).to.deep.equal({}); request.respondWith({ status: 204, @@ -913,7 +929,6 @@ describe('ajax', () => { expect(request.url).to.equal('/flibbertyJibbet'); expect(request.requestHeaders).to.deep.equal({ 'content-type': 'application/json;charset=utf-8', - 'x-requested-with': 'XMLHttpRequest', }); request.respondWith({ @@ -1026,9 +1041,7 @@ describe('ajax', () => { async: true, body: undefined, crossDomain: true, - headers: { - 'x-requested-with': 'XMLHttpRequest', - }, + headers: {}, includeDownloadProgress: true, method: 'GET', responseType: 'json', @@ -1154,9 +1167,7 @@ describe('ajax', () => { async: true, body: undefined, crossDomain: true, - headers: { - 'x-requested-with': 'XMLHttpRequest', - }, + headers: {}, includeUploadProgress: true, includeDownloadProgress: true, method: 'GET', diff --git a/src/internal/ajax/ajax.ts b/src/internal/ajax/ajax.ts index e54e2cbd4c..722d9f21fa 100644 --- a/src/internal/ajax/ajax.ts +++ b/src/internal/ajax/ajax.ts @@ -279,7 +279,20 @@ export function fromAjax(config: AjaxConfig): Observable> { // Here we're pulling off each of the configuration arguments // that we don't want to add to the request information we're // passing around. - const { queryParams, body: configuredBody, headers: configuredHeaders, ...remainingConfig } = config; + const configWithDefaults = { + // Default values + async: true, + crossDomain: true, + withCredentials: false, + method: 'GET', + timeout: 0, + responseType: 'json' as XMLHttpRequestResponseType, + + // Override with passed user values + ...config, + }; + + const { queryParams, body: configuredBody, headers: configuredHeaders, ...remainingConfig } = configWithDefaults; let { url } = remainingConfig; if (!url) { @@ -334,7 +347,7 @@ export function fromAjax(config: AjaxConfig): Observable> { // None of this is necessary, it's only being set because it's "the thing libraries do" // Starting back as far as JQuery, and continuing with other libraries such as Angular 1, // Axios, et al. - if (!config.crossDomain && !('x-requested-with' in headers)) { + if (!configWithDefaults.crossDomain && !('x-requested-with' in headers)) { headers['x-requested-with'] = 'XMLHttpRequest'; } @@ -353,16 +366,8 @@ export function fromAjax(config: AjaxConfig): Observable> { const body = extractContentTypeAndMaybeSerializeBody(configuredBody, headers); const _request: AjaxRequest = { - // Default values - async: true, - crossDomain: true, - withCredentials: false, - method: 'GET', - timeout: 0, - responseType: 'json' as XMLHttpRequestResponseType, - - // Override with passed user values - ...remainingConfig, + // Take the final configuration + ...configWithDefaults, // Set values we ensured above url,