8000 fix: adjust the order of t:urlDecodeUni and t:utf8toUnicode in 941160 PL1 by syinwu · Pull Request #3450 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: adjust the order of t:urlDecodeUni and t:utf8toUnicode in 941160 PL1 #3450

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.

Sign up for GitHub

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
Closed
wants to merge 2 commits into from
Closed

fix: adjust the order of t:urlDecodeUni and t:utf8toUnicode in 941160 PL1 #3450

wants to merge 2 commits into from

Conversation

syinwu
Copy link
@syinwu syinwu commented Dec 27, 2023 •
edited by azurit
Loading

Adjusting the order of t:urlDecodeUni and t:utf8toUnicode transformations in rule 941160 to prevent FPs.

Fixes #3449.

@azurit
Copy link
Member
azurit commented Dec 27, 2023

Hi @Bxlxx. Thank for this PR. Can you, please, correctly set subject and description?

@syinwu syinwu changed the title fix: #3449 fix: adjust the order of t:urlDecodeUni and t:utf8toUnicode in 941160 PL1 Dec 27, 2023
@azurit
Copy link
Member
azurit commented Dec 27, 2023

changelog comment:

feat: Adjusting the order of transformations in rule 941160 (bxlxx)

@dune73
Copy link
Member
dune73 commented Dec 31, 2023

This is a change that needs to be discussed with regards to other rules as well. Let's hold back until we have discussed this.

@dune73 dune73 added the ⚠️ do not merge Additional work or discussion is needed despite passing tests label Dec 31, 2023
@dune73 dune73 mentioned this pull request Jan 8, 2024
Closed
@dune73 dune73 mentioned this pull request Feb 5, 2024
Closed
@dune73
Copy link
Member
dune73 commented Mar 2, 2024

Adding this to the agenda for meeting next Monday.

@dune73 dune73 mentioned this pull request Mar 4, 2024
Closed
@fzipi
Copy link
Member
fzipi commented Apr 1, 2024

BTW, no decision was taken in the March meeting. No one volunteered to take a more holistic look on the order of transformation yet.

@fzipi
Copy link
Member
fzipi commented Apr 1, 2024

@dune73 From what I see in the original report, wouldn't this be the expected sequence for transformations? If this is fixing something, why don't we merge this one first and then create a new issue to track doing this for the rest?

@dune73 dune73 mentioned this pull request Apr 1, 2024
Closed
@dune73
Copy link
Member
dune73 commented Apr 2, 2024

Here is my interpretation of what is happening:

Old: utf8toUnicode,t:urlDecodeUni

/index.html´%2F%2E%2E%2F -> /index.html%u2019%2F%2E%2E%2F -> /index.html´/../

New: t:urlDecodeUni,t:utf8toUnicode

/index.html´%2F%2E%2E%2F -> /index.html´/../ -> /index.html%u2019/../

If this interpretation is correct, then I do not see the PR as the desired behavior.

@github-actions github-actions bot added the Stale label May 3, 2024
@dune73 dune73 mentioned this pull request May 6, 2024
Closed
@github-actions github-actions bot closed this May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Stale ⚠️ do not merge Additional work or discussion is needed despite passing tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

URL encoding FPs with rule 941160 PL1
4 participants
@syinwu @azurit @dune73 @fzipi
0