Skip to content
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

Ajax CORS fixes #6696

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 28 additions & 8 deletions spec/observables/dom/ajax-spec.ts
Expand Up @@ -129,6 +129,7 @@ describe('ajax', () => {
url: '/some/path',
xsrfCookieName: 'foo',
xsrfHeaderName: 'Custom-Header-Name',
crossDomain: false, // this is the default anyway
};

ajax(obj).subscribe();
Expand Down Expand Up @@ -188,12 +189,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',
Expand Down Expand Up @@ -912,7 +934,7 @@ describe('ajax', () => {
expect(request.method).to.equal('PATCH');
expect(request.url).to.equal('/flibbertyJibbet');
expect(request.requestHeaders).to.deep.equal({
'content-type': 'application/json;charset=utf-8',
'content-type': 'application/json;charset=utf-8',
'x-requested-with': 'XMLHttpRequest',
});

Expand Down Expand Up @@ -1153,10 +1175,8 @@ describe('ajax', () => {
const request = {
async: true,
body: undefined,
crossDomain: true,
headers: {
'x-requested-with': 'XMLHttpRequest',
},
crossDomain: false,
headers: {},
includeUploadProgress: true,
includeDownloadProgress: true,
method: 'GET',
Expand Down
46 changes: 28 additions & 18 deletions src/internal/ajax/ajax.ts
Expand Up @@ -276,12 +276,31 @@ const LOAD = 'load';

export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
return new Observable((destination) => {
// 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 requestConfig = {
// Default values
async: true,
crossDomain: false,
withCredentials: false,
method: 'GET',
timeout: 0,
responseType: 'json' as XMLHttpRequestResponseType,

// Override with passed user values
...config,
} as const;

const {
queryParams,
body: configuredBody,
headers: configuredHeaders,
withCredentials,
xsrfCookieName,
xsrfHeaderName,
crossDomain,
} = requestConfig;

let url = requestConfig.url;

let { url } = remainingConfig;
if (!url) {
throw new TypeError('url is required');
}
Expand Down Expand Up @@ -334,14 +353,13 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
// 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 (!crossDomain && !('x-requested-with' in headers)) {
headers['x-requested-with'] = 'XMLHttpRequest';
}

// Allow users to provide their XSRF cookie name and the name of a custom header to use to
// send the cookie.
const { withCredentials, xsrfCookieName, xsrfHeaderName } = remainingConfig;
if ((withCredentials || !remainingConfig.crossDomain) && xsrfCookieName && xsrfHeaderName) {
if ((withCredentials || !crossDomain) && xsrfCookieName && xsrfHeaderName) {
const xsrfCookie = document?.cookie.match(new RegExp(`(^|;\\s*)(${xsrfCookieName})=([^;]*)`))?.pop() ?? '';
if (xsrfCookie) {
headers[xsrfHeaderName] = xsrfCookie;
Expand All @@ -353,16 +371,8 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
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
...requestConfig,

// Set values we ensured above
url,
Expand Down