Skip to content

[Tests] Handle escape char correctly in wildcard patterns #69850

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

Merged
merged 2 commits into from
Mar 3, 2021

Conversation

ywangd
Copy link
Member

@ywangd ywangd commented Mar 3, 2021

Escape char is handled specially while building automaton. It should also be handled when buidling the target text for matching.

Resolves: #69837

@ywangd ywangd added >test Issues or PRs that are addressing/adding tests :Security/Security Security issues without another label v8.0.0 v7.13.0 labels Mar 3, 2021
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Mar 3, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@tvernum tvernum self-requested a review March 3, 2021 04:55
@@ -26,10 +26,11 @@ public void testEmptySet() throws Exception {
public void testSingleWildcard() throws Exception {
final String prefix = randomAlphaOfLengthBetween(3, 5);
final StringMatcher matcher = StringMatcher.of(prefix + "*");
final String normalisedPrefix = prefix.replace("\\", "");
Copy link
Contributor

Choose a reason for hiding this comment

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

prefix cannot contain a \, it's alphabetic only.

@@ -48,10 +49,11 @@ public void testUnicodeWildcard() throws Exception {
final String prefix = randomValueOtherThanMany(StringMatcherTests::hasHighSurrogate,
() -> randomRealisticUnicodeOfLengthBetween(3, 5));
final StringMatcher matcher = StringMatcher.of(prefix + "*");
final String normalisedPrefix = prefix.replace("\\", "");
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just push this up into the otherThanMany check.

randomValueOtherThanMany(StringMatcherTests::hasHighSurrogateOrBackslash

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated as suggested.
PS: I think we do need some tests for the behaviour of the escape char. This is not the right place for it. Maybe when fixing #69851.

@ywangd ywangd requested a review from tvernum March 3, 2021 05:04
@ywangd ywangd merged commit 52b7b7a into elastic:master Mar 3, 2021
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Mar 3, 2021
)

Escape char is handled specially while building automaton. It should also be handled 
when buidling the target text for matching.
ywangd added a commit that referenced this pull request Mar 3, 2021
…69852)

Escape char is handled specially while building automaton. It should also be handled 
when buidling the target text for matching.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Security Security issues without another label Team:Security Meta label for security team >test Issues or PRs that are addressing/adding tests v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StringMatcherTests.testUnicodeWildcard test failure
4 participants