Increase the randomness and size of the sessionId (newId()) #1391 (#1392

)

README.md

@@ -277,6 +277,7 @@ Most configuration fields are named such that they can be defaulted to falsey. A
 | disableInstrumentaionKeyValidation | false | If true, instrumentation key validation check is bypassed. Default value is false.
 | enablePerfMgr | false | [Optional] When enabled (true) this will create local perfEvents for code that has been instrumented to emit perfEvents (via the doPerf() helper). This can be used to identify performance issues within the SDK based on your usage or optionally within your own instrumented code. [More details are available by the basic documentation](./docs/PerformanceMonitoring.md). Since v2.5.7
 | perfEvtsSendAll | false | [Optional] When _enablePerfMgr_ is enabled and the [IPerfManager](https://github.com/microsoft/ApplicationInsights-JS/blob/master/shared/AppInsightsCore/src/JavaScriptSDK.Interfaces/IPerfManager.ts) fires a [INotificationManager](https://github.com/microsoft/ApplicationInsights-JS/blob/master/shared/AppInsightsCore/src/JavaScriptSDK.Interfaces/INotificationManager.ts).perfEvent() this flag determines whether an event is fired (and sent to all listeners) for all events (true) or only for 'parent' events (false &lt;default&gt;).<br />A parent [IPerfEvent](https://github.com/microsoft/ApplicationInsights-JS/blob/master/shared/AppInsightsCore/src/JavaScriptSDK.Interfaces/IPerfEvent.ts) is an event where no other IPerfEvent is still running at the point of this event being created and it's _parent_ property is not null or undefined. Since v2.5.7
+| idLength | 22 | [Optional] Identifies the default length used to generate new random session and user id's. Defaults to 22, previous default value was 5 (v2.5.8 or less), if you need to keep the previous maximum length you should set this value to 5.
 
 ## Single Page Applications
 

extensions/applicationinsights-analytics-js/src/JavaScriptSDK/Telemetry/PageViewManager.ts

@@ -117,9 +117,11 @@ export class PageViewManager {
 
                 // if the performance timing is supported by the browser, calculate the custom duration
                 const start = pageViewPerformanceManager.getPerformanceTiming().navigationStart;
-                customDuration = DateTimeUtils.GetDuration(start, +new Date);
-                if (!pageViewPerformanceManager.shouldCollectDuration(customDuration)) {
-                    customDuration = undefined;
+                if (start > 0) {
+                    customDuration = DateTimeUtils.GetDuration(start, +new Date);
+                    if (!pageViewPerformanceManager.shouldCollectDuration(customDuration)) {
+                        customDuration = undefined;
+                    }
                 }
 
                 // if the user has provided duration, send a page view telemetry with the provided duration. Otherwise, if
@@ -185,7 +187,7 @@ export class PageViewManager {
                                     pageViewPerformanceSent = true;
                                 }
                             }
-                        } else if (DateTimeUtils.GetDuration(start, +new Date) > maxDurationLimit) {
+                        } else if (start > 0 && DateTimeUtils.GetDuration(start, +new Date) > maxDurationLimit) {
                             // if performance timings are not ready but we exceeded the maximum duration limit, just log a page view telemetry
                             // with the maximum duration limit. Otherwise, keep waiting until performance timings are ready
                             processed = true;

extensions/applicationinsights-analytics-js/src/JavaScriptSDK/Telemetry/PageViewPerformanceManager.ts

@@ -163,7 +163,7 @@ export class PageViewPerformanceManager {
         } else {
             // for other page views, don't report if it's outside of a reasonable range
             for (let i = 0; i < durations.length; i++) {
-                if (durations[i] >= this.MAX_DURATION_ALLOWED) {
+                if (durations[i] < 0 || durations[i] >= this.MAX_DURATION_ALLOWED) {
                     return false;
                 }
             }

extensions/applicationinsights-properties-js/Tests/Selenium/properties.tests.ts

@@ -1,6 +1,6 @@
 /// <reference path="../TestFramework/TestClass.ts" />
 
-import { AppInsightsCore, IConfiguration, DiagnosticLogger, ITelemetryItem } from "@microsoft/applicationinsights-core-js";
+import { AppInsightsCore, IConfiguration, DiagnosticLogger, ITelemetryItem, CoreUtils } from "@microsoft/applicationinsights-core-js";
 import PropertiesPlugin from "../../src/PropertiesPlugin";
 import { ITelemetryConfig } from "../../src/Interfaces/ITelemetryConfig";
 import { Util, IWeb } from "@microsoft/applicationinsights-common";
@@ -142,29 +142,35 @@ export class PropertiesTests extends TestClass {
                 // setup
                 var actualCookieName: string;
                 var actualCookieValue: string;
-                var newIdStub = this.sandbox.stub(Util, "newId", () => "newId");
-                var getCookieStub = this.sandbox.stub(Util, "getCookie", () => "");
-                var setCookieStub = this.sandbox.stub(Util, "setCookie", (logger, cookieName, cookieValue) => {
-                    actualCookieName = cookieName;
-                    actualCookieValue = cookieValue;
-                });
-
-                // act
-                this.properties.initialize(this.getEmptyConfig(), this.core, []);
-
-                // verify
-                Assert.equal("ai_user", actualCookieName, "ai_user cookie is set");
-                var cookieValueParts = actualCookieValue.split(';');
-
-                Assert.equal(2, cookieValueParts.length, "ai_user cookie value should have actual value and expiration");
-                Assert.equal(2, cookieValueParts[0].split('|').length, "ai_user cookie value before expiration should include user id and acq date");
-                Assert.equal("newId", cookieValueParts[0].split('|')[0], "First part of ai_user cookie value should be new user id guid");
-                Assert.equal(new Date().toString(), (new Date(cookieValueParts[0].split('|')[1])).toString(), "Second part of ai_user cookie should be parsable as date");
-
-                var expiration = cookieValueParts[1];
-                Assert.equal(true, expiration.substr(0, "expires=".length) === "expires=", "ai_user cookie expiration part should start with expires=");
-                var expirationDate = new Date(expiration.substr("expires=".length));
-                Assert.equal(true, expirationDate > (new Date), "ai_user cookie expiration should be in the future");
+                let newIdPrev = CoreUtils.newId;
+                try {
+                    // Not using sinon stub as it's not restoring the previous version properly (getting newId is not a function for tests run after this one)
+                    CoreUtils.newId = () => "newId";
+                    var getCookieStub = this.sandbox.stub(Util, "getCookie", () => "");
+                    var setCookieStub = this.sandbox.stub(Util, "setCookie", (logger, cookieName, cookieValue) => {
+                        actualCookieName = cookieName;
+                        actualCookieValue = cookieValue;
+                    });
+
+                    // act
+                    this.properties.initialize(this.getEmptyConfig(), this.core, []);
+
+                    // verify
+                    Assert.equal("ai_user", actualCookieName, "ai_user cookie is set");
+                    var cookieValueParts = actualCookieValue.split(';');
+
+                    Assert.equal(2, cookieValueParts.length, "ai_user cookie value should have actual value and expiration");
+                    Assert.equal(2, cookieValueParts[0].split('|').length, "ai_user cookie value before expiration should include user id and acq date");
+                    Assert.equal("newId", cookieValueParts[0].split('|')[0], "First part of ai_user cookie value should be new user id guid");
+                    Assert.equal(new Date().toString(), (new Date(cookieValueParts[0].split('|')[1])).toString(), "Second part of ai_user cookie should be parsable as date");
+
+                    var expiration = cookieValueParts[1];
+                    Assert.equal(true, expiration.substr(0, "expires=".length) === "expires=", "ai_user cookie expiration part should start with expires=");
+                    var expirationDate = new Date(expiration.substr("expires=".length));
+                    Assert.equal(true, expirationDate > (new Date), "ai_user cookie expiration should be in the future");
+                } finally {
+                    CoreUtils.newId = newIdPrev;
+                }
             }
         });
 
@@ -175,29 +181,35 @@ export class PropertiesTests extends TestClass {
                 var id = "userId"
                 var actualCookieName: string;
                 var actualCookieValue: string;
-                var newIdStub = this.sandbox.stub(Util, "newId", () => "newId");
-                var getCookieStub = this.sandbox.stub(Util, "getCookie", () => "");
-                var setCookieStub = this.sandbox.stub(Util, "setCookie", (logger, cookieName, cookieValue) => {
-                    actualCookieName = cookieName;
-                    actualCookieValue = cookieValue;
-                });
-
-                // act
-                this.properties.initialize(this.getEmptyConfig(), this.core, []);
-
-                // verify
-                Assert.equal("ai_user", actualCookieName, "ai_user cookie is set");
-                var cookieValueParts = actualCookieValue.split(';');
-
-                Assert.equal(2, cookieValueParts.length, "ai_user cookie value should have actual value and expiration");
-                Assert.equal(2, cookieValueParts[0].split('|').length, "ai_user cookie value before expiration should include user id and acq date");
-                Assert.equal("newId", cookieValueParts[0].split('|')[0], "First part of ai_user cookie value should be new user id guid");
-                Assert.equal(new Date().toString(), (new Date(cookieValueParts[0].split('|')[1])).toString(), "Second part of ai_user cookie should be parsable as date");
-
-                var expiration = cookieValueParts[1];
-                Assert.equal(true, expiration.substr(0, "expires=".length) === "expires=", "ai_user cookie expiration part should start with expires=");
-                var expirationDate = new Date(expiration.substr("expires=".length));
-                Assert.equal(true, expirationDate > (new Date), "ai_user cookie expiration should be in the future");
+                let newIdPrev = CoreUtils.newId;
+                try {
+                    // Not using sinon stub as it's not restoring the previous version properly (getting newId is not a function for tests run after this one)
+                    CoreUtils.newId = () => "newId";
+                    var getCookieStub = this.sandbox.stub(Util, "getCookie", () => "");
+                    var setCookieStub = this.sandbox.stub(Util, "setCookie", (logger, cookieName, cookieValue) => {
+                        actualCookieName = cookieName;
+                        actualCookieValue = cookieValue;
+                    });
+
+                    // act
+                    this.properties.initialize(this.getEmptyConfig(), this.core, []);
+
+                    // verify
+                    Assert.equal("ai_user", actualCookieName, "ai_user cookie is set");
+                    var cookieValueParts = actualCookieValue.split(';');
+
+                    Assert.equal(2, cookieValueParts.length, "ai_user cookie value should have actual value and expiration");
+                    Assert.equal(2, cookieValueParts[0].split('|').length, "ai_user cookie value before expiration should include user id and acq date");
+                    Assert.equal("newId", cookieValueParts[0].split('|')[0], "First part of ai_user cookie value should be new user id guid");
+                    Assert.equal(new Date().toString(), (new Date(cookieValueParts[0].split('|')[1])).toString(), "Second part of ai_user cookie should be parsable as date");
+
+                    var expiration = cookieValueParts[1];
+                    Assert.equal(true, expiration.substr(0, "expires=".length) === "expires=", "ai_user cookie expiration part should start with expires=");
+                    var expirationDate = new Date(expiration.substr("expires=".length));
+                    Assert.equal(true, expirationDate > (new Date), "ai_user cookie expiration should be in the future");
+                } finally {
+                    CoreUtils.newId = newIdPrev;
+                }
             }
         });
 
@@ -662,7 +674,8 @@ export class PropertiesTests extends TestClass {
             sdkExtension: () => "",
             isBrowserLinkTrackingEnabled: () => true,
             appId: () => "",
-            namePrefix: () => ""
+            namePrefix: () => "",
+            idLength: () => 22
         }
     }
 }

extensions/applicationinsights-properties-js/src/Context/Session.ts

@@ -1,14 +1,15 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import { ISession, Util, DateTimeUtils } from '@microsoft/applicationinsights-common';
+import { ISession, Util } from '@microsoft/applicationinsights-common';
 import { IDiagnosticLogger, _InternalMessageId, LoggingSeverity, CoreUtils, DiagnosticLogger } from '@microsoft/applicationinsights-core-js';
 
 export interface ISessionConfig {
     sessionRenewalMs?: () => number;
     sessionExpirationMs?: () => number;
     cookieDomain?: () => string;
     namePrefix?: () => string;
+    idLength?: () => number;
 }
 
 export class Session implements ISession {
@@ -18,8 +19,8 @@ export class Session implements ISession {
     public id?: string;
 
     /**
-     * The date at which this guid was genereated.
-     * Per the spec the ID will be regenerated if more than acquisitionSpan milliseconds ellapse from this time.
+     * The date at which this guid was generated.
+     * Per the spec the ID will be regenerated if more than acquisitionSpan milliseconds elapsed from this time.
      */
     public acquisitionDate?: number;
 
@@ -74,12 +75,14 @@ export class _SessionManager {
             this.initializeAutomaticSession();
         }
 
-        const now = DateTimeUtils.Now();
+        // Always using Date getTime() as there is a bug in older IE instances that causes the performance timings to have the hi-bit set eg 0x800000000 causing
+        // the number to be incorrect.
+        const now = new Date().getTime();
 
         const acquisitionExpired = this.config.sessionExpirationMs() === 0 ? false : now - this.automaticSession.acquisitionDate > this.config.sessionExpirationMs();
         const renewalExpired = this.config.sessionExpirationMs() === 0 ? false : now - this.automaticSession.renewalDate > this.config.sessionRenewalMs();
 
-        // renew if acquisitionSpan or renewalSpan has ellapsed
+        // renew if acquisitionSpan or renewalSpan has elapsed
         if (acquisitionExpired || renewalExpired) {
             // update automaticSession so session state has correct id
             this.renew();
@@ -126,7 +129,7 @@ export class _SessionManager {
     }
 
     /**
-     *  Extract id, aquisitionDate, and renewalDate from an ai_session payload string and
+     *  Extract id, acquisitionDate, and renewalDate from an ai_session payload string and
      *  use this data to initialize automaticSession.
      *
      *  @param {string} sessionData - The string stored in an ai_session cookie or local storage backup
@@ -166,9 +169,9 @@ export class _SessionManager {
     }
 
     private renew() {
-        const now = DateTimeUtils.Now();
+        const now = new Date().getTime();
 
-        this.automaticSession.id = Util.newId();
+        this.automaticSession.id = Util.newId((this.config && this.config.idLength) ? this.config.idLength() : 22);
         this.automaticSession.acquisitionDate = now;
         this.automaticSession.renewalDate = now;
 
@@ -196,15 +199,15 @@ export class _SessionManager {
             cookieExpiry.setTime(renewalExpiry);
         }
 
-        const cookieDomnain = this.config.cookieDomain ? this.config.cookieDomain() : null;
+        const cookieDomain = this.config.cookieDomain ? this.config.cookieDomain() : null;
 
         // if sessionExpirationMs is set to 0, it means the expiry is set to 0 for this session cookie
         // A cookie with 0 expiry in the session cookie will never expire for that browser session.  If the browser is closed the cookie expires.  
         // Another browser instance does not inherit this cookie.
         const UTCString = this.config.sessionExpirationMs() === 0 ? '0' : cookieExpiry.toUTCString();
-        Util.setCookie(this._logger, this._storageNamePrefix(), cookie.join('|') + ';expires=' + UTCString, cookieDomnain);
+        Util.setCookie(this._logger, this._storageNamePrefix(), cookie.join('|') + ';expires=' + UTCString, cookieDomain);
 
-        this.cookieUpdatedTimestamp = DateTimeUtils.Now();
+        this.cookieUpdatedTimestamp = new Date().getTime();
     }
 
     private setStorage(guid: string, acq: number, renewal: number) {

extensions/applicationinsights-properties-js/src/Context/User.ts

@@ -61,7 +61,7 @@ export class User implements IUser {
         this.config = config;
 
         if (!this.id) {
-            this.id = Util.newId();
+            this.id = CoreUtils.newId(config && config.idLength ? config.idLength() : 22);
             const date = new Date();
             const acqStr = CoreUtils.toISOString(date);
             this.accountAcquisitionDate = acqStr;

extensions/applicationinsights-properties-js/src/Interfaces/ITelemetryConfig.ts

@@ -12,4 +12,5 @@ export interface ITelemetryConfig {
     isBrowserLinkTrackingEnabled: () => boolean;
     appId: () => string;
     namePrefix: () => string;
+    idLength: () => number;
 }

extensions/applicationinsights-properties-js/src/PropertiesPlugin.ts

@@ -26,7 +26,8 @@ export default class PropertiesPlugin extends BaseTelemetryPlugin implements IPr
             sdkExtension: () => null,
             isBrowserLinkTrackingEnabled: () => false,
             appId: () => null,
-            namePrefix: () => undefined
+            namePrefix: () => undefined,
+            idLength: () => 22
         }
         return defaultConfig;
     }