Skip to content

Disable httpHeadersOverwriteParam for ReactNative to address #703. #717

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 2, 2018
Merged

Disable httpHeadersOverwriteParam for ReactNative to address #703. #717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
078604e
Disable httpHeadersOverwriteParam for ReactNative to address #703.
Apr 23, 2018
9c5fadf
CR feedback (improve comments, use existing isReactNative() method).
May 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/firestore/src/local/simple_db.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export class SimpleDb {
// https://developer.microsoft.com/en-us/microsoft-edge/platform/status/indexeddbarraysandmultientrysupport/

// Check the UA string to find out the browser.
// TODO(mikelehen): Move this logic into packages/util/environment.ts
const ua = window.navigator.userAgent;

// IE 10
Expand Down
29 changes: 25 additions & 4 deletions packages/firestore/src/platform_browser/webchannel_connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
createWebChannelTransport
} from '@firebase/webchannel-wrapper';

import { isReactNative } from '@firebase/util';

import { Token } from '../api/credentials';
import { DatabaseId, DatabaseInfo } from '../core/database_info';
import { SDK_VERSION } from '../core/version';
Expand Down Expand Up @@ -201,10 +203,6 @@ export class WebChannelConnection implements Connection {
// parameter.
httpSessionIdParam: 'gsessionid',
initMessageHeaders: {},
// Send our custom headers as a '$httpHeaders=' url param to avoid CORS
// preflight round-trip. This is formally defined here:
// https://github.com/google/closure-library/blob/b0e1815b13fb92a46d7c9b3c30de5d6a396a3245/closure/goog/net/rpc/httpcors.js#L40
httpHeadersOverwriteParam: '$httpHeaders',
messageUrlParams: {
// This param is used to improve routing and project isolation by the
// backend and must be included in every request.
Expand All @@ -215,7 +213,30 @@ export class WebChannelConnection implements Connection {
sendRawJson: true,
supportsCrossDomainXhr: true
};

this.modifyHeadersForRequest(request.initMessageHeaders, token);

// Sending the custom headers we just added to request.initMessageHeaders
// (Authorization, etc.) will trigger the browser to make a CORS preflight
// request because the XHR will no longer meet the criteria for a "simple"
// CORS request:
// https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#Simple_requests
//
// Therefore to avoid the CORS preflight request (an extra network
// roundtrip), we use the httpHeadersOverwriteParam option to specify that
// the headers should instead be encoded into a special "$httpHeaders" query
// parameter, which is recognized by the webchannel backend. This is
// formally defined here:
// https://github.com/google/closure-library/blob/b0e1815b13fb92a46d7c9b3c30de5d6a396a3245/closure/goog/net/rpc/httpcors.js#L32
//
// But for some unclear reason (see
// https://github.com/firebase/firebase-js-sdk/issues/703), this breaks
// ReactNative and so we exclude it, which just means ReactNative may be
// subject to the extra network roundtrip for CORS preflight.
if (!isReactNative()) {
request['httpHeadersOverwriteParam'] = '$httpHeaders';
}

const url = urlParts.join('');
log.debug(LOG_TAG, 'Creating WebChannel: ' + url + ' ' + request);
// tslint:disable-next-line:no-any Because listen isn't defined on it.
Expand Down