From e13a6da8e6f30057970ccb459c655d18163419df Mon Sep 17 00:00:00 2001 From: "Richard Park (WDG/ES)" Date: Wed, 11 Dec 2019 15:45:39 -0800 Subject: [PATCH 1/2] Use the x-ms-useragent header when in the browser. Also, whitelisted the x-ms-useragent header for logging as well since it's the User-Agent header we use for browsers. --- .../src/appConfigurationClient.ts | 89 +++++++++++++------ .../{synctokenpolicy.spec.ts => http.spec.ts} | 23 ++++- sdk/core/core-http/lib/policies/logPolicy.ts | 1 + 3 files changed, 83 insertions(+), 30 deletions(-) rename sdk/appconfiguration/app-configuration/test/internal/{synctokenpolicy.spec.ts => http.spec.ts} (90%) diff --git a/sdk/appconfiguration/app-configuration/src/appConfigurationClient.ts b/sdk/appconfiguration/app-configuration/src/appConfigurationClient.ts index a99f2f5efc90..b883ef1df700 100644 --- a/sdk/appconfiguration/app-configuration/src/appConfigurationClient.ts +++ b/sdk/appconfiguration/app-configuration/src/appConfigurationClient.ts @@ -12,6 +12,7 @@ import { isTokenCredential, exponentialRetryPolicy, systemErrorRetryPolicy, + ServiceClientCredentials, } from "@azure/core-http"; import { throttlingRetryPolicy } from "./policies/throttlingRetryPolicy"; import { TokenCredential } from "@azure/identity"; @@ -48,9 +49,9 @@ import { transformKeyValue, formatAcceptDateTime } from "./internal/helpers"; -import { tracingPolicy } from "@azure/core-http"; +import { tracingPolicy, isNode as isNodeFromCoreHttp } from "@azure/core-http"; import { Spanner } from "./internal/tracingHelpers"; -import { GetKeyValuesResponse, AppConfigurationOptions } from "./generated/src/models"; +import { GetKeyValuesResponse, AppConfigurationOptions as GeneratedAppConfigurationClientOptions } from "./generated/src/models"; import { syncTokenPolicy, SyncTokens } from './internal/synctokenpolicy'; const apiVersion = "1.0"; @@ -82,6 +83,11 @@ export interface InternalAppConfigurationClientOptions extends AppConfigurationC * NOTE: this is an internal option, not for general client usage. */ syncTokens?: SyncTokens; + /** + * Whether we want to run as if we're in node or in the browser. + * (currently only affects which name we use for the user agent header) + */ + isNodeOverride?: boolean; } /** @@ -110,31 +116,22 @@ export class AppConfigurationClient { tokenCredentialOrOptions?: TokenCredential | AppConfigurationClientOptions, options?:AppConfigurationClientOptions ) { + let appConfigOptions: InternalAppConfigurationClientOptions = {}; + let appConfigCredential: ServiceClientCredentials | TokenCredential; + let appConfigEndpoint: string; + if (isTokenCredential(tokenCredentialOrOptions)) { - const syncTokens = - (options && (options as InternalAppConfigurationClientOptions).syncTokens) || - new SyncTokens(); - - this.client = new AppConfiguration( - tokenCredentialOrOptions, - apiVersion, - getAppConfigurationOptions(connectionStringOrEndpoint, syncTokens) - ); + appConfigOptions = (options as InternalAppConfigurationClientOptions) || {}; + appConfigCredential = tokenCredentialOrOptions; + appConfigEndpoint = connectionStringOrEndpoint; } else { - const syncTokens = - (tokenCredentialOrOptions && - (tokenCredentialOrOptions as InternalAppConfigurationClientOptions).syncTokens) || - new SyncTokens(); + appConfigOptions = (tokenCredentialOrOptions as InternalAppConfigurationClientOptions) || {}; const regexMatch = connectionStringOrEndpoint.match(ConnectionStringRegex); - if (regexMatch) { - const appConfigCredential = new AppConfigCredential(regexMatch[2], regexMatch[3]); - this.client = new AppConfiguration( - appConfigCredential, - apiVersion, - getAppConfigurationOptions(regexMatch[1], syncTokens) - ); + if (regexMatch) { + appConfigCredential = new AppConfigCredential(regexMatch[2], regexMatch[3]); + appConfigEndpoint = regexMatch[1]; } else { throw new Error( `Invalid connection string. Valid connection strings should match the regex '${ConnectionStringRegex.source}'.` @@ -142,6 +139,14 @@ export class AppConfigurationClient { } } + const syncTokens = appConfigOptions.syncTokens || new SyncTokens(); + + this.client = new AppConfiguration( + appConfigCredential, + apiVersion, + getGeneratedClientOptions(appConfigEndpoint, syncTokens, appConfigOptions) + ); + this.spanner = new Spanner("Azure.Data.AppConfiguration", "appconfig"); } @@ -475,14 +480,20 @@ export class AppConfigurationClient { } } -function getAppConfigurationOptions( +/** + * Gets the options for the generated AppConfigurationClient + * @internal + * @ignore + */ +export function getGeneratedClientOptions( baseUri: string, syncTokens: SyncTokens, -): AppConfigurationOptions { - + internalAppConfigOptions: InternalAppConfigurationClientOptions +): GeneratedAppConfigurationClientOptions { + const retryPolicies = [ exponentialRetryPolicy(), - systemErrorRetryPolicy(), + systemErrorRetryPolicy(), throttlingRetryPolicy() ]; @@ -495,8 +506,30 @@ function getAppConfigurationOptions( tracingPolicy(), syncTokenPolicy(syncTokens), ...retryPolicies, - ...defaults, - ] + ...defaults, + ], + userAgentHeaderName: getUserAgentHeaderName(internalAppConfigOptions) }; } +/** + * @ignore + * @internal + */ +export function getUserAgentHeaderName(internalAppConfigOptions: InternalAppConfigurationClientOptions) { + const isNode = internalAppConfigOptions.isNodeOverride == null + ? isNodeFromCoreHttp + : internalAppConfigOptions.isNodeOverride; + + let userAgentHeaderName: string | undefined; + + if (!isNode) { + // we only need to override this when we're in the browser + // where we're (mostly) not allowed to override the User-Agent + // header. + userAgentHeaderName = "x-ms-useragent"; + } + + return userAgentHeaderName; +} + diff --git a/sdk/appconfiguration/app-configuration/test/internal/synctokenpolicy.spec.ts b/sdk/appconfiguration/app-configuration/test/internal/http.spec.ts similarity index 90% rename from sdk/appconfiguration/app-configuration/test/internal/synctokenpolicy.spec.ts rename to sdk/appconfiguration/app-configuration/test/internal/http.spec.ts index f05e700201b2..051eb3bea506 100644 --- a/sdk/appconfiguration/app-configuration/test/internal/synctokenpolicy.spec.ts +++ b/sdk/appconfiguration/app-configuration/test/internal/http.spec.ts @@ -9,8 +9,9 @@ import * as assert from "assert"; import { AppConfigurationClient } from "../../src"; const nock = require("nock"); import { createAppConfigurationClientForTests, assertThrowsRestError, getTokenAuthenticationCredential } from "../testHelpers"; +import { getUserAgentHeaderName } from '../../src/appConfigurationClient'; -describe("sync tokens", () => { +describe("http request related tests", () => { describe("unit tests", () => { describe("parseSyncToken", () => { it("can parse various sync tokens", () => { @@ -30,6 +31,20 @@ describe("sync tokens", () => { }); }); + it("useragentheadername", () => { + assert.ok(getUserAgentHeaderName({ + isNodeOverride: true + }) === undefined); + + // since we're only running these tests in node this will be the same as the + // case above (undefined, thus using the normal User-Agent header) + assert.ok(getUserAgentHeaderName({}) === undefined); + + assert.equal(getUserAgentHeaderName({ + isNodeOverride: false + }), "x-ms-useragent"); + }); + describe("syncTokens", () => { it("basic", () => { const syncTokens = new SyncTokens(); @@ -73,7 +88,7 @@ describe("sync tokens", () => { // properly extracting and sending the sync token header (which is // why they appear to not do much of anything meaningful with what // they send or reply back with). - describe("request/reply tests for header", () => { + describe("request/reply tests for sync token headers", () => { let client: AppConfigurationClient; let syncTokens: SyncTokens; let scope: any; @@ -214,6 +229,10 @@ describe("sync tokens", () => { assert.equal(syncTokens.getSyncTokenHeaderValue(), 'clearReadOnly=value'); }); }); + + describe("proper user agent header name is used", () => { + + }); }); function splitAndSort(syncTokens: string | undefined): string { diff --git a/sdk/core/core-http/lib/policies/logPolicy.ts b/sdk/core/core-http/lib/policies/logPolicy.ts index ec8354691c70..6002bfdffc35 100644 --- a/sdk/core/core-http/lib/policies/logPolicy.ts +++ b/sdk/core/core-http/lib/policies/logPolicy.ts @@ -39,6 +39,7 @@ const RedactedString = "REDACTED"; const defaultAllowedHeaderNames = [ "x-ms-client-request-id", "x-ms-return-client-request-id", + "x-ms-useragent", "traceparent", "Accept", From 23ab0116c204867cd7f6fec69cf8dafdcafa998e Mon Sep 17 00:00:00 2001 From: Richard Park Date: Thu, 12 Dec 2019 09:42:27 -0800 Subject: [PATCH 2/2] Remove empty unneeded test --- .../app-configuration/test/internal/http.spec.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sdk/appconfiguration/app-configuration/test/internal/http.spec.ts b/sdk/appconfiguration/app-configuration/test/internal/http.spec.ts index 051eb3bea506..c18b8f7ef26e 100644 --- a/sdk/appconfiguration/app-configuration/test/internal/http.spec.ts +++ b/sdk/appconfiguration/app-configuration/test/internal/http.spec.ts @@ -229,10 +229,6 @@ describe("http request related tests", () => { assert.equal(syncTokens.getSyncTokenHeaderValue(), 'clearReadOnly=value'); }); }); - - describe("proper user agent header name is used", () => { - - }); }); function splitAndSort(syncTokens: string | undefined): string {