X Tutup
The Wayback Machine - https://web.archive.org/web/20230304010007/https://github.com/github/codeql/pull/12333
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

Jump to bottom

ReflectedXss: Prevent bad join order #12333

Merged
merged 1 commit into from Mar 1, 2023
Merged

ReflectedXss: Prevent bad join order #12333

merged 1 commit into from Mar 1, 2023

Conversation

kaspersv
Copy link
Contributor

@kaspersv kaspersv commented Feb 28, 2023 •
edited

The Core team is planning to make some changes to the join orderer that causes a join order regression for the isLocalHeaderDefinition predicate. This PR adds pragmas to prevent the compiler from picking the bad join order.

@github-actions github-actions bot added the JS label Feb 28, 2023
@kaspersv kaspersv changed the title ReflectedXss: Fix join order regression ReflectedXss: Prevent bad join order Feb 28, 2023
@kaspersv kaspersv marked this pull request as ready for review March 1, 2023 11:36
@kaspersv kaspersv requested a review from a team as a code owner March 1, 2023 11:36
@kaspersv kaspersv requested review from aibaars and removed request for a team March 1, 2023 11:36
@kaspersv kaspersv added the no-change-note-required This PR does not need a change note label Mar 1, 2023
erik-krogh
erik-krogh approved these changes Mar 1, 2023
View changes
Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

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

Looks very reasonable.

And I assume you've benchmarked that it doesn't cause a regression with the current join-orderer.

@erik-krogh erik-krogh removed the request for review from aibaars March 1, 2023 11:39
@kaspersv
Copy link
Contributor Author

kaspersv commented Mar 1, 2023

Yes, see the referenced DCA experiment.

@erik-krogh erik-krogh merged commit 64dad3d into github:main Mar 1, 2023
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erik-krogh erik-krogh erik-krogh approved these changes

Assignees
No one assigned
Labels
JS no-change-note-required This PR does not need a change note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kaspersv @erik-krogh
X Tutup