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

[BUG] Can't include Correlation Header on IE can fail #1260 #1266

Merged
merged 1 commit into from May 20, 2020
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
3 changes: 2 additions & 1 deletion channels/applicationinsights-channel-js/src/Sender.ts
Expand Up @@ -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();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using local var to save space for minification.

if (!ieVer || ieVer > 9) {
this.diagLog().throwInternal(
LoggingSeverity.CRITICAL,
_InternalMessageId.TransmissionFailed,
Expand Down
8 changes: 3 additions & 5 deletions extensions/applicationinsights-dependencies-js/src/ajax.ts
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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(': ');
Expand Down
327 changes: 327 additions & 0 deletions shared/AppInsightsCommon/Tests/Util.tests.ts

Large diffs are not rendered by default.

95 changes: 75 additions & 20 deletions shared/AppInsightsCommon/src/Util.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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/")) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding trident as not all IE userAgents always include MSIE

let tridentVer = parseInt(myNav.split("trident/")[1]);
if (tridentVer) {
return tridentVer + 4;
}
}

return null;
}

/**
Expand Down Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There was a test error with just using the single instance which caused the tests to occasionally fail -- as the used value kept getting overwritten.

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 {
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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;
}
Expand Down
11 changes: 11 additions & 0 deletions shared/AppInsightsCore/src/JavaScriptSDK/CoreUtils.ts
Expand Up @@ -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.
Expand Down
14 changes: 11 additions & 3 deletions tools/rollup-es3/src/es3/Es3Tokens.ts
Expand Up @@ -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();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updating this to support an issue that Xiao was having while attempt to update the version of TypeScript which caused the CoreUtils reference to become CoreUtils$0.toISOString()

"Utils.toISOString" // or if it's a reference to Utils.isISOString()
]
},
{
Expand Down Expand Up @@ -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 ],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add check for string trim() which is not supported on ES3

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()
]
}
];

4 changes: 2 additions & 2 deletions tools/rollup-es3/src/es3/Interfaces.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
Expand Down
16 changes: 12 additions & 4 deletions tools/rollup-es3/src/es3/Utils.ts
Expand Up @@ -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;
}
}
}
}
}
Expand Down