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

Fix bug in wildcard filter where pattern would match in the middle of a string #2076

Merged
merged 3 commits into from
Apr 7, 2022

Conversation

romac
Copy link
Member

@romac romac commented Apr 7, 2022

Closes: #2075

Description

The fix anchors the regex by delimiting it with ^ and $ so that it only matches the full string and not anywhere in the middle.


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

romac added 2 commits April 7, 2022 08:54
… a string

The fix anchors the regex by delimiting it with `^` and `$` so that
it only matches the full string and not anywhere in the middle.
@romac romac requested a review from hu55a1n1 April 7, 2022 06:56
@romac romac requested review from adizere and ancazamfir as code owners April 7, 2022 06:56
Copy link
Member

@hu55a1n1 hu55a1n1 left a comment

Choose a reason for hiding this comment

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

Thanks for the super-quick fix @romac! Nicely done! 🙏


impl Wildcard {
pub fn new(pattern: String) -> Result<Self, regex::Error> {
let escaped = regex::escape(&pattern).replace("\\*", "(?:.*)");
let regex = format!("^{escaped}$").parse()?;
Copy link
Member

Choose a reason for hiding this comment

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

Nice use of captured identifiers! 💯

@@ -137,28 +137,35 @@ impl Serialize for ChannelFilters {

/// Newtype wrapper for expressing wildcard patterns compiled to a [`regex::Regex`].
#[derive(Clone, Debug)]
pub struct Wildcard(regex::Regex);
pub struct Wildcard {
pattern: String,
Copy link
Member

Choose a reason for hiding this comment

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

👌

@romac romac merged commit 0690b8d into master Apr 7, 2022
@romac romac deleted the romac/2075-fix-wildcard branch April 7, 2022 10:19
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
… a string (informalsystems#2076)

* Fix bug in wildcard filter where pattern would match in the middle of a string

The fix anchors the regex by delimiting it with `^` and `$` so that
it only matches the full string and not anywhere in the middle.

* Add changelog entry

* Store original pattern inside `Wildcard` instead of re-deriving it from the regex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wildcard based packet filtering isn't working as expected
2 participants