Skip to content

Conversation

@asger-semmle
Copy link
Contributor

This works around an issue in extraction of TypeScript tokens, where we get an incorrect token stream in some cases. The issue doesn't affect the AST so only queries that specifically rely on tokens are affected.

It's too late to get the proper fix into 1.18, so doing this quick fix for 1.18, and then a proper fix later for 1.19.

@asger-semmle asger-semmle requested a review from a team as a code owner September 4, 2018 12:22
Copy link

@xiemaisi xiemaisi left a comment

Choose a reason for hiding this comment

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

LGTM as a temporary workaround, but needs a change note.

| Missing rate limiting | More true-positive results, fewer false-positive results | This rule now recognizes additional rate limiters and expensive route handlers. |
| Missing X-Frame-Options HTTP header | Fewer false-positive results | This rule now treats header names case-insensitively. |
| Reflected cross-site scripting | Fewer false-positive results | This rule now treats header names case-insensitively. |
| Semicolon insertion | Fewer results | This rule now ignores TypeScript files as it did not work correctly. |
Copy link

Choose a reason for hiding this comment

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

Perhaps also mention the other rules? Also, we may want to emphasise that they are only disabled temporarily.

@felicitymay felicitymay added this to the 1.18 milestone Sep 4, 2018
@xiemaisi xiemaisi added the JS label Sep 5, 2018
@felicitymay
Copy link
Contributor

This looks as if it may be ready to merge.

@semmle-qlci semmle-qlci merged commit 62e9946 into github:rc/1.18 Sep 5, 2018
aibaars pushed a commit that referenced this pull request Oct 14, 2021
Create `ast_node_parent` relation
smowton pushed a commit to smowton/codeql that referenced this pull request Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants