Skip to content

Add regular expression support in header match#335

Merged
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
amalgam8:header_regex
Jan 10, 2017
Merged

Add regular expression support in header match#335
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
amalgam8:header_regex

Conversation

@rshriram
Copy link
Copy Markdown
Member

@rshriram rshriram commented Jan 9, 2017

fixes #199

This PR adds support for C++ style regexes while matching request headers. There are no config syntax changes.

@rshriram
Copy link
Copy Markdown
Member Author

rshriram commented Jan 9, 2017

@mattklein123

@mattklein123
Copy link
Copy Markdown
Member

I would like people to understand that they are using regex, so I think we should actually opt-in via config.

What do you think about having a parameter in the header match, "regex" which defaults to false, and if set to true does a regex match otherwise does what we do today?

@rshriram
Copy link
Copy Markdown
Member Author

rshriram commented Jan 9, 2017

I would definitely be up for that. Should be easy to add as well. Do we need to do some additional scrubbing on the incoming request headers before passing it via a regex_match ? If so, are there any best practices or will the existing c++ regex subsystem suffice to handle super large headers, etc. ?
( @louiscryan ^^^ )

@mattklein123
Copy link
Copy Markdown
Member

mattklein123 commented Jan 9, 2017

@rshriram my knowledge of regex security is close to zero, so will definitely defer to others on best practices around scrubbing, etc. For now I think as long as it is opt-in and clearly documented we can proceed with a basic implementation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you link out to the grammar: http://en.cppreference.com/w/cpp/regex/ecmascript

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same below

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know you didn't do this, but:

cfg_header_data.value_.empty()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: can you do regex case last since it will be less common, so invert logic and do !cfg_header_data.is_regex_

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit is_regex_

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit regex_pattern_

@mattklein123 mattklein123 merged commit 29cb758 into envoyproxy:master Jan 10, 2017
enricoschiattarella pushed a commit to enricoschiattarella/envoy that referenced this pull request Jan 10, 2017
@rshriram rshriram deleted the header_regex branch January 30, 2017 02:39
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
* Revert "Upgrade gRPC to v1.3.4 (envoyproxy#335)"

This reverts commit f9bf1d9.

* Revert "Update gRPC to v1.3.2 (envoyproxy#315)"

This reverts commit 6f8953d.
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
Automatic merge from submit-queue.

[DO NOT MERGE] Auto PR to update dependencies of mixerclient

This PR will be merged automatically once checks are successful.
```release-note
none
```
jplevyak referenced this pull request in jplevyak/envoy Jan 22, 2020
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
mathetake added a commit that referenced this pull request Mar 3, 2026
**Commit Message**

Previously, the .bin directly was not cleared, so this fixes it.

**Related Issues/PRs (if applicable)**

Follow up on #329

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
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.

Support for regular expression parsing in header match

2 participants