[BUG] Can include Correlation Header on IE can fail #1260 (#1266)

[BUG] ajax.ts is using string trim() which is not supported on IE7/8 #1251

channels/applicationinsights-channel-js/src/Sender.ts

@@ -423,7 +423,8 @@ export class Sender extends BaseTelemetryPlugin implements IChannelControlsAI {
             this._retryAt = null;
         } catch (e) {
             /* Ignore this error for IE under v10 */
-            if (!Util.getIEVersion() || Util.getIEVersion() > 9) {
+            let ieVer = Util.getIEVersion();
+            if (!ieVer || ieVer > 9) {
                 this.diagLog().throwInternal(
                     LoggingSeverity.CRITICAL,
                     _InternalMessageId.TransmissionFailed,

extensions/applicationinsights-dependencies-js/src/ajax.ts

@@ -59,10 +59,8 @@ function _supportsAjaxMonitoring(ajaxMonitorInstance:AjaxMonitor): boolean {
             !_isNullOrUndefined(proto.abort);
     }
 
-    // disable in IE8 or older (https://www.w3schools.com/jsref/jsref_trim_string.asp)
-    try {
-        " a ".trim();
-    } catch (ex) {
+    let ieVer = Util.getIEVersion();
+    if (ieVer && ieVer < 9) {
         result = false;
     }
 
@@ -671,7 +669,7 @@ export class AjaxMonitor extends BaseTelemetryPlugin implements IDependenciesPlu
                                 if (headers) {
                                     // xhr.getAllResponseHeaders() method returns all the response headers, separated by CRLF, as a string or null
                                     // the regex converts the header string into an array of individual headers
-                                    const arr = headers.trim().split(/[\r\n]+/);
+                                    const arr = CoreUtils.strTrim(headers).split(/[\r\n]+/);
                                     const responseHeaderMap = {};
                                     _arrForEach(arr, (line) => {
                                         const parts = line.split(': ');

shared/AppInsightsCommon/src/Util.ts

@@ -474,10 +474,7 @@ export class Util {
     /**
      * helper method to trim strings (IE8 does not implement String.prototype.trim)
      */
-    public static trim(str: any): string {
-        if (!_isString(str)) { return str; }
-        return str.replace(/^\s+|\s+$/g, "");
-    }
+    public static trim = CoreUtils.strTrim;
 
     /**
      * generate random id string
@@ -553,11 +550,20 @@ export class Util {
     public static toISOStringForIE8 = CoreUtils.toISOString;
 
     /**
-     * Gets IE version if we are running on IE, or null otherwise
+     * Gets IE version returning the document emulation mode if we are running on IE, or null otherwise
      */
     public static getIEVersion(userAgentStr: string = null): number {
         const myNav = userAgentStr ? userAgentStr.toLowerCase() : (_navigator ? (_navigator.userAgent ||"").toLowerCase() : "");
-        return (myNav.indexOf('msie') !== -1) ? parseInt(myNav.split('msie')[1]) : null;
+        if (myNav.indexOf("msie") !== -1) {
+            return parseInt(myNav.split("msie")[1]);
+        } else if (myNav.indexOf("trident/")) {
+            let tridentVer = parseInt(myNav.split("trident/")[1]);
+            if (tridentVer) {
+                return tridentVer + 4;
+            }
+        }
+
+        return null;
     }
 
     /**
@@ -652,16 +658,35 @@ export class Util {
 
 export class UrlHelper {
     private static document: any = getDocument()||{};
-    private static htmlAnchorElement: HTMLAnchorElement;
+
+    private static _htmlAnchorIdx: number = 0;
+    // Use an array of temporary values as it's possible for multiple calls to parseUrl() will be called with different URLs
+    // Using a cache size of 5 for now as it current depth usage is at least 2, so adding a minor buffer to handle future updates
+    private static _htmlAnchorElement: HTMLAnchorElement[] = [null, null, null, null, null];
 
     public static parseUrl(url: string): HTMLAnchorElement {
-        if (!UrlHelper.htmlAnchorElement) {
-            UrlHelper.htmlAnchorElement = !!UrlHelper.document.createElement ? UrlHelper.document.createElement('a') : { host: UrlHelper.parseHost(url) }; // fill host field in the fallback case as that is the only externally required field from this fn
+        let anchorIdx = UrlHelper._htmlAnchorIdx;
+        let anchorCache = UrlHelper._htmlAnchorElement;
+        let tempAnchor = anchorCache[anchorIdx];
+        if (!UrlHelper.document.createElement) {
+            // Always create the temp instance if createElement is not available
+            tempAnchor = { host: UrlHelper.parseHost(url, true) } as HTMLAnchorElement;
+        } else if (!anchorCache[anchorIdx]) {
+            // Create and cache the unattached anchor instance 
+            tempAnchor = anchorCache[anchorIdx] = UrlHelper.document.createElement('a');
         }
 
-        UrlHelper.htmlAnchorElement.href = url;
+        tempAnchor.href = url;
 
-        return UrlHelper.htmlAnchorElement;
+        // Move the cache index forward
+        anchorIdx++;
+        if (anchorIdx >= anchorCache.length) {
+            anchorIdx = 0;
+        }
+
+        UrlHelper._htmlAnchorIdx = anchorIdx;
+
+        return tempAnchor;
     }
 
     public static getAbsoluteUrl(url: string): string {
@@ -693,15 +718,43 @@ export class UrlHelper {
     }
 
     // Fallback method to grab host from url if document.createElement method is not available
-    public static parseHost(url: string) {
+    public static parseHost(url: string, inclPort?: boolean) {
+        let fullHost = UrlHelper.parseFullHost(url, inclPort);
+        if (fullHost ) {
+            const match = fullHost.match(/(www[0-9]?\.)?(.[^/:]+)(\:[\d]+)?/i);
+            if (match != null && match.length > 3 && _isString(match[2]) && match[2].length > 0) {
+                return match[2] + (match[3] ||"");
+            }
+        }
+
+        return fullHost;
+    }
+
+    /**
+     * Get the full host from the url, optionally including the port
+     */
+    public static parseFullHost(url: string, inclPort?: boolean) {
+        let result = null;
         if (url) {
-            const match = url.match(/:\/\/(www[0-9]?\.)?(.[^/:]+)/i);
+            const match = url.match(/(\w*):\/\/(.[^/:]+)(\:[\d]+)?/i);
             if (match != null && match.length > 2 && _isString(match[2]) && match[2].length > 0) {
-                return match[2];
+                result = match[2] || "";
+                if (inclPort && match.length > 2) {
+                    const protocol = (match[1] || "").toLowerCase();
+                    let port = match[3] || "";
+                    // IE includes the standard port so pass it off if it's the same as the protocol
+                    if (protocol === "http" && port === ":80") {
+                        port = "";
+                    } else if (protocol === "https" && port === ":443") {
+                        port = "";
+                    }
+
+                    result += port;
+                }
             }
         }
 
-        return null;
+        return result;
     }
 }
 
@@ -711,20 +764,22 @@ export class CorrelationIdHelper {
     /**
      * Checks if a request url is not on a excluded domain list and if it is safe to add correlation headers.
      * Headers are always included if the current domain matches the request domain. If they do not match (CORS),
-     * they are regexed across correlationHeaderDomains and correlationHeaderExcludedDomains to determine if headers are included.
+     * they are regex-ed across correlationHeaderDomains and correlationHeaderExcludedDomains to determine if headers are included.
      * Some environments don't give information on currentHost via window.location.host (e.g. Cordova). In these cases, the user must
      * manually supply domains to include correlation headers on. Else, no headers will be included at all.
      */
     public static canIncludeCorrelationHeader(config: ICorrelationConfig, requestUrl: string, currentHost?: string) {
-        if (config && config.disableCorrelationHeaders) {
+        if (!requestUrl || (config && config.disableCorrelationHeaders)) {
             return false;
         }
 
-        if (!requestUrl) {
-            return false;
+        let requestHost = UrlHelper.parseUrl(requestUrl).host.toLowerCase();
+        if (requestHost && (requestHost.indexOf(":443") !== -1 || requestHost.indexOf(":80") !== -1)) {
+            // [Bug #1260] IE can include the port even for http and https URLs so if present 
+            // try and parse it to remove if it matches the default protocol port
+            requestHost = (UrlHelper.parseFullHost(requestUrl, true) || "").toLowerCase();
         }
 
-        const requestHost = UrlHelper.parseUrl(requestUrl).host.toLowerCase();
         if ((!config || !config.enableCorsCorrelation) && requestHost !== currentHost) {
             return false;
         }

shared/AppInsightsCore/src/JavaScriptSDK/CoreUtils.ts

@@ -295,6 +295,17 @@ export class CoreUtils {
         return value;
     }
 
+    /**
+     * helper method to trim strings (IE8 does not implement String.prototype.trim)
+     */
+    public static strTrim(str: any): string {
+        if (!CoreUtils.isString(str)) {
+            return str; 
+        }
+
+        return str.replace(/^\s+|\s+$/g, "");
+    }
+
     /**
      * Creates an object that has the specified prototype, and that optionally contains specified properties. This helper exists to avoid adding a polyfil
      * for older browsers that do not define Object.create eg. ES3 only, IE8 just in case any page checks for presence/absence of the prototype implementation.

tools/rollup-es3/src/es3/Es3Tokens.ts

@@ -153,11 +153,11 @@ export const defaultEs3CheckTokens:IEs3CheckKeyword[] = [
         ]  
     },
     {
-        funcNames: [ /([\w0-9]*)\.toISOString[\s]*\(/g ],
+        funcNames: [ /([\w0-9\$]*)\.toISOString[\s]*\(/g ],
         errorMsg: "[%funcName%] is not supported in an ES3 environment, use CoreUtils.toISOString()",
         ignoreFuncMatch: [ 
-            "CoreUtils.toISOString",         // Make sure this isn't a reference to CoreUtils.isISOString()
-            "Utils.toISOString"              // or if it's a reference to Utils.isISOString()
+            /CoreUtils(\$[\d]+)+\.toISOString/,     // Make sure this isn't a reference to CoreUtils.isISOString(); CoreUtils$1.isISOString();
+            "Utils.toISOString"                     // or if it's a reference to Utils.isISOString()
         ]
     },
     {
@@ -190,6 +190,14 @@ export const defaultEs3CheckTokens:IEs3CheckKeyword[] = [
     {
         funcNames: [ /[\s\(,][gs]et[\s]+([\w]+)[\s]*\(\)[\s]*\{/g ],
         errorMsg: "[%funcName%] is not supported in an ES3 environment."
+    },
+    {
+        funcNames: [ /([\w0-9]*)\.(trim)[\s]*\(/g ],
+        errorMsg: "[%funcName%] is not a supported string method in an ES3 environment, use CoreUtils.strTrim().",
+        ignoreFuncMatch: [ 
+            "Util.trim",                            // Make sure this isn't a reference to Util.trim()
+            "DataSanitizer.trim"                    // Make sure this isn't a reference to Util.trim()
+        ]
     }
 ];
 

tools/rollup-es3/src/es3/Interfaces.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 
 /**
- * Because of the test infrastructure (PhamtonJS) the RegEx can't use the "s" flag (gis vs gi) or named groups
+ * Because of the test infrastructure (PhantomJS) the RegEx can't use the "s" flag (gis vs gi) or named groups
  */
 export interface INamedGroups {
     name: string,
@@ -35,7 +35,7 @@ export interface IEs3CheckKeyword {
      *  A Set of strings to use for matching funcNames to ignore, this is required because of infra (build) issues
      * with negative lookbehind, internally this uses indexOf() to provide a partial existence check.
      */
-    ignoreFuncMatch?:string[],
+    ignoreFuncMatch?:Array<string|RegExp>,
 
     /**
      * The prefix added to any reported error, defaults to "Invalid ES3 function"

tools/rollup-es3/src/es3/Utils.ts

@@ -57,12 +57,20 @@ export function isIgnore(id:string, keyword:IEs3CheckKeyword, isTransform:boolea
 
 export function isIgnoreFuncMatch(funcMatch:string, keyword:IEs3CheckKeyword) {
     let result = false;
-    if (keyword.ignoreFuncMatch) {
+    if (funcMatch && keyword.ignoreFuncMatch) {
         for (let ignoreIdx in keyword.ignoreFuncMatch) {
             let ignoreMatch = keyword.ignoreFuncMatch[ignoreIdx];
-            if (funcMatch && funcMatch.indexOf(ignoreMatch) !== -1) {
-                result = true;
-                break;
+            if (ignoreMatch) {
+                if (typeof ignoreMatch === "string" && funcMatch.indexOf(ignoreMatch) !== -1) {
+                    result = true;
+                    break;
+                } else if (ignoreMatch instanceof RegExp) {
+                    let match = ignoreMatch.exec(funcMatch);
+                    if (match && match.length > 0) {
+                        result = true;
+                        break;
+                    }
+                }
             }
         }
     }