Skip to content

Commit

Permalink
Avoid matching polyfilled global names in NameEngine.
Browse files Browse the repository at this point in the history
Fix #22.

PiperOrigin-RevId: 354387758
Change-Id: I659f36dd0eb608aa252a908f6271a2fafd126191
  • Loading branch information
uraj committed Jan 28, 2021
1 parent 3c528e0 commit 1d9047d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
7 changes: 0 additions & 7 deletions src/third_party/tsetse/util/absolute_matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,6 @@ export class AbsoluteMatcher {
debugLog(() => `start matching ${n.getText()} in ${p?.getText()}`);

if (p !== undefined) {
// Do not match global symbols appearing on the LHS of an assignment
// expression. This avoids false positives in polyfill code.
if (this.filePath === GLOBAL && ts.isBinaryExpression(p) &&
p.left === n && p.operatorToken.kind === ts.SyntaxKind.EqualsToken) {
return false;
}

// Check if the node is being declared. Declaration may be imported
// without programmer being aware of. We should not alert them about that.
// Since import statments are also declarations, this have two notable
Expand Down
15 changes: 15 additions & 0 deletions src/third_party/tsetse/util/ast_tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,21 @@ export function shouldExamineNode(n: ts.Node) {
isInStockLibraries(n));
}

/**
* Returns the top-level property access expression (or element access
* expression) that the input node is part of.
*
* For example, given an expression `c` which is part of `a['b'].c = 1;`,
* the function returns the whole LHS expression `a['b'].c`.
*/
export function walkUpPropertyAndElementAccess(n: ts.Node): ts.Node {
while (ts.isPropertyAccessExpression(n.parent) ||
ts.isElementAccessExpression(n.parent)) {
n = n.parent;
}
return n;
}

/**
* Return whether the given Node is (or is in) a library included as default.
* We currently look for a node_modules/typescript/ prefix, but this could
Expand Down
22 changes: 18 additions & 4 deletions src/third_party/tsetse/util/pattern_engines/name_engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as ts from 'typescript';

import {Checker} from '../../checker';
import {AbsoluteMatcher} from '../absolute_matcher';
import {walkUpPropertyAndElementAccess} from '../ast_tools';
import {isExpressionOfAllowedTrustedType} from '../is_trusted_type';
import {TrustedTypesConfig} from '../trusted_types_configuration';

Expand All @@ -21,19 +22,32 @@ function isCalledWithAllowedTrustedType(
return false;
}

function isPolyfill(n: ts.Node, matcher: AbsoluteMatcher) {
if (matcher.filePath === 'GLOBAL') {
const wholeExp = walkUpPropertyAndElementAccess(n);
const parent = wholeExp.parent;
if (parent && ts.isBinaryExpression(parent) && parent.left === wholeExp &&
parent.operatorToken.kind === ts.SyntaxKind.EqualsToken) {
return true;
}
}
return false;
}

function checkIdentifierNode(
tc: ts.TypeChecker, n: ts.Identifier, matcher: AbsoluteMatcher,
allowedTrustedType: TrustedTypesConfig|undefined): ts.Node|undefined {
if (isPolyfill(n, matcher)) return;
if (!matcher.matches(n, tc)) return;
if (isCalledWithAllowedTrustedType(tc, n, allowedTrustedType)) return;


return n;
}

function checkElementAccessNode(
tc: ts.TypeChecker, n: ts.ElementAccessExpression, matcher: AbsoluteMatcher,
allowedTrustedType: TrustedTypesConfig|undefined): ts.Node|undefined {
if (isPolyfill(n, matcher)) return;
if (!matcher.matches(n.argumentExpression, tc)) return;
if (isCalledWithAllowedTrustedType(tc, n, allowedTrustedType)) return;

Expand All @@ -46,9 +60,9 @@ export class NameEngine extends PatternEngine {
for (const value of this.config.values) {
const matcher = new AbsoluteMatcher(value);

// `String.prototype.split` only returns emtpy array when both the string
// and the splitter are empty. Here we should be able to safely assert pop
// returns a non-null result.
// `String.prototype.split` only returns emtpy array when both the
// string and the splitter are empty. Here we should be able to safely
// assert pop returns a non-null result.
const bannedIdName = matcher.bannedName.split('.').pop()!;
checker.onNamedIdentifier(
bannedIdName,
Expand Down

0 comments on commit 1d9047d

Please sign in to comment.