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

fix: feature detection to check for browser #738

Merged
merged 3 commits into from Jun 20, 2019
Merged
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
6 changes: 2 additions & 4 deletions package.json
Expand Up @@ -24,8 +24,7 @@
"gcp-metadata": "^2.0.0",
"gtoken": "^3.0.0",
"jws": "^3.1.5",
"lru-cache": "^5.0.0",
"semver": "^6.0.0"
"lru-cache": "^5.0.0"
},
"devDependencies": {
"@compodoc/compodoc": "^1.1.7",
Expand All @@ -39,7 +38,6 @@
"@types/ncp": "^2.0.1",
"@types/nock": "^10.0.3",
"@types/node": "^10.5.1",
"@types/semver": "^6.0.0",
"@types/sinon": "^7.0.0",
"@types/tmp": "^0.1.0",
"assert-rejects": "^1.0.0",
Expand Down Expand Up @@ -68,7 +66,7 @@
"ncp": "^2.0.0",
"nock": "^10.0.0",
"null-loader": "^3.0.0",
"nyc": "^14.0.0",
"nyc": "^14.1.1",
"prettier": "^1.13.4",
"puppeteer": "^1.11.0",
"sinon": "^7.0.0",
Expand Down
1 change: 0 additions & 1 deletion src/auth/authclient.ts
Expand Up @@ -18,7 +18,6 @@ import {EventEmitter} from 'events';
import {GaxiosOptions, GaxiosPromise} from 'gaxios';

import {DefaultTransporter} from '../transporters';

import {Credentials} from './credentials';

export declare interface AuthClient {
Expand Down
1 change: 0 additions & 1 deletion src/auth/googleauth.ts
Expand Up @@ -23,7 +23,6 @@ import * as path from 'path';
import * as stream from 'stream';

import {createCrypto} from '../crypto/crypto';
import {isBrowser} from '../isbrowser';
import * as messages from '../messages';
import {DefaultTransporter, Transporter} from '../transporters';

Expand Down
1 change: 0 additions & 1 deletion src/auth/jwtaccess.ts
Expand Up @@ -19,7 +19,6 @@ import * as LRU from 'lru-cache';
import * as stream from 'stream';

import * as messages from '../messages';

import {JWTInput} from './credentials';
import {Headers, RequestMetadataResponse} from './oauth2client';

Expand Down
5 changes: 2 additions & 3 deletions src/auth/oauth2client.ts
Expand Up @@ -23,8 +23,7 @@ import {
import * as querystring from 'querystring';
import * as stream from 'stream';

import {createCrypto, JwkCertificate} from '../crypto/crypto';
import {isBrowser} from '../isbrowser';
import {createCrypto, JwkCertificate, hasBrowserCrypto} from '../crypto/crypto';
import * as messages from '../messages';
import {BodyResponseCallback} from '../transporters';

Expand Down Expand Up @@ -1015,7 +1014,7 @@ export class OAuth2Client extends AuthClient {

async getFederatedSignonCertsAsync(): Promise<FederatedSignonCertsResponse> {
const nowTime = new Date().getTime();
const format: CertificateFormat = isBrowser()
const format = hasBrowserCrypto()
? CertificateFormat.JWK
: CertificateFormat.PEM;
if (
Expand Down
5 changes: 1 addition & 4 deletions src/crypto/browser/crypto.ts
Expand Up @@ -19,13 +19,10 @@

import * as base64js from 'base64-js';

import {isBrowser} from '../../isbrowser';
import {CryptoSigner} from '../crypto';

// Not all browsers support `TextEncoder`. The following `require` will
// provide a fast UTF8-only replacement for those browsers that don't support
// text encoding natively.
if (isBrowser() && typeof TextEncoder === 'undefined') {
if (typeof TextEncoder === 'undefined') {
require('fast-text-encoding');
}

Expand Down
11 changes: 9 additions & 2 deletions src/crypto/crypto.ts
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import {isBrowser} from '../isbrowser';
import {BrowserCrypto} from './browser/crypto';
import {NodeCrypto} from './node/crypto';

Expand Down Expand Up @@ -56,8 +55,16 @@ export interface Crypto {
}

export function createCrypto(): Crypto {
if (isBrowser()) {
if (hasBrowserCrypto()) {
return new BrowserCrypto();
}
return new NodeCrypto();
}

export function hasBrowserCrypto() {
return (
typeof window !== 'undefined' &&
typeof window.crypto !== 'undefined' &&
typeof window.crypto.subtle !== 'undefined'
);
}
2 changes: 1 addition & 1 deletion src/crypto/node/crypto.ts
Expand Up @@ -15,7 +15,7 @@
*/

import * as crypto from 'crypto';
import {Crypto, CryptoSigner} from '../crypto';
import {Crypto} from '../crypto';

export class NodeCrypto implements Crypto {
async sha256DigestBase64(str: string): Promise<string> {
Expand Down
19 changes: 0 additions & 19 deletions src/isbrowser.ts

This file was deleted.

11 changes: 2 additions & 9 deletions src/messages.ts
Expand Up @@ -14,9 +14,6 @@
* limitations under the License.
*/

import * as semver from 'semver';
import {isBrowser} from './isbrowser';

export enum WarningTypes {
WARNING = 'Warning',
DEPRECATION = 'DeprecationWarning',
Expand All @@ -28,18 +25,14 @@ export function warn(warning: Warning) {
return;
}
warning.warned = true;
if (isBrowser()) {
console.warn(warning.message);
} else if (semver.satisfies(process.version, '>=8')) {
if (typeof process !== 'undefined' && process.emitWarning) {
// @types/node doesn't recognize the emitWarning syntax which
// accepts a config object, so `as any` it is
// https://nodejs.org/docs/latest-v8.x/api/process.html#process_process_emitwarning_warning_options
// tslint:disable-next-line no-any
process.emitWarning(warning.message, warning as any);
} else {
// This path can be removed once we drop support for Node 6.
// https://nodejs.org/docs/latest-v6.x/api/process.html#process_process_emitwarning_warning_name_ctor
process.emitWarning(warning.message, warning.type);
console.warn(warning.message);
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/transporters.ts
Expand Up @@ -21,8 +21,6 @@ import {
GaxiosResponse,
request,
} from 'gaxios';

import {isBrowser} from './isbrowser';
import {validate} from './options';

// tslint:disable-next-line no-var-requires
Expand Down Expand Up @@ -60,7 +58,7 @@ export class DefaultTransporter {
*/
configure(opts: GaxiosOptions = {}): GaxiosOptions {
opts.headers = opts.headers || {};
if (!isBrowser()) {
if (typeof window === 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

we're okay to keep this check the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aye. In this case, it doesn't have an impact on which browser feature we're trying to use. It's just a naive check so we can try to send the right HTTP headers (I think).

// set transporter user agent if not in browser
const uaValue: string = opts.headers['User-Agent'];
if (!uaValue) {
Expand Down
1 change: 0 additions & 1 deletion system-test/test.kitchen.ts
Expand Up @@ -15,7 +15,6 @@
*/

import * as assert from 'assert';
import * as cp from 'child_process';
import * as execa from 'execa';
import * as fs from 'fs';
import * as mv from 'mv';
Expand Down
1 change: 0 additions & 1 deletion test/test.compute.ts
Expand Up @@ -20,7 +20,6 @@ import {BASE_PATH, HEADERS, HOST_ADDRESS} from 'gcp-metadata';
import * as nock from 'nock';
import * as sinon from 'sinon';
import {Compute} from '../src';
import * as qs from 'querystring';

nock.disableNetConnect();

Expand Down
1 change: 0 additions & 1 deletion test/test.crypto.ts
@@ -1,4 +1,3 @@
import * as nativeCrypto from 'crypto';
import * as fs from 'fs';
import {assert} from 'chai';
import {createCrypto} from '../src/crypto/crypto';
Expand Down