Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add regex for path matching #3874

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

Open
fabian4 wants to merge 1 commit into nginx:main
base: main
Choose a base branch
Loading
from fabian4:add_regex_for_path_matching

Conversation

Copy link
Contributor

@fabian4 fabian4 commented Sep 8, 2025 •
edited
Loading

Proposed changes

Problem: Add regex for path matching

external (regex path match without any head/method/queryparams condition)

exact: /foo => = /foo
prefix: /foo => = /foo + ^~ /foo/
regex: /foo/(.*)$ => ~ /foo/(.*)$

Closes #3110

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Add regex support for path matching

Copy link

nginx-bot bot commented Sep 8, 2025

Hi @fabian4!

Thanks for opening this pull request!
Be sure to check out our Contributing Guidelines while you wait for someone on the team to review this.

@fabian4 fabian4 marked this pull request as draft September 8, 2025 15:12
@fabian4 fabian4 force-pushed the add_regex_for_path_matching branch 2 times, most recently from 2202934 to 72227ea Compare September 9, 2025 15:28
@sindhushiv sindhushiv added this to the v2.2.0 milestone Sep 10, 2025
@sindhushiv sindhushiv moved this from 🆕 New to External Pull Requests in NGINX Gateway Fabric Sep 10, 2025
Copy link
Contributor

Hey @fabian4, thanks a lot for your contribution to NGINX Gateway Fabric!

I noticed there are still some parts left in this PR—were you looking for initial feedback before continuing development, or are you planning to submit this in parts?
fabian4 reacted with heart emoji

Copy link
Contributor Author

fabian4 commented Sep 12, 2025

Hi @salonichf5, thanks a lot for the comment! I kept this PR as a draft since there are still some parts I’d like to finish up. In the meantime, getting some early feedback would be super helpful — that way I can adjust things early and make sure I’m heading in the right direction. Really appreciate your input!
salonichf5 reacted with thumbs up emoji

@fabian4 fabian4 force-pushed the add_regex_for_path_matching branch from 72227ea to 54135b5 Compare September 12, 2025 15:14
@github-project-automation github-project-automation bot moved this from External Pull Requests to ✅ Done in NGINX Gateway Fabric Sep 12, 2025
@fabian4 fabian4 reopened this Sep 12, 2025
@sjberman sjberman moved this from 🆕 New to External Pull Requests in NGINX Gateway Fabric Sep 12, 2025
@fabian4 fabian4 force-pushed the add_regex_for_path_matching branch from c76e809 to 63c06fe Compare September 14, 2025 04:47
Copy link
Contributor

Hi @salonichf5, thanks a lot for the comment! I kept this PR as a draft since there are still some parts I’d like to finish up. In the meantime, getting some early feedback would be super helpful — that way I can adjust things early and make sure I’m heading in the right direction. Really appreciate your input!

For sure, we will get some eyes on it. Thank you again for the contribution.
fabian4 reacted with heart emoji

@fabian4 fabian4 marked this pull request as ready for review September 22, 2025 06:58
@sjberman sjberman added the enhancement New feature or request label Sep 22, 2025
Copy link
Collaborator

@fabian4 We'll be sure to review this soon!

In the meantime, I ran the pipeline and there are some linting/unit test errors. Also, please be sure to follow this guide for formatting your main commit message and PR description. And please update the release note section the PR description as well, so we can include a notein our CHANGELOG when this ultimately gets released.
fabian4 reacted with heart emoji

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds regex support for path matching in NGINX Gateway Fabric, extending the existing exact and prefix path matching capabilities. It introduces a new PathTypeRegularExpression type and implements the necessary validation and configuration logic to support regex-based routing.

Key changes:

  • Adds validation infrastructure for regex path patterns with RE2 compatibility
  • Updates dataplane configuration to handle regex path types
  • Modifies NGINX config generation to create regex location blocks with ~ prefix

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/controller/state/validation/validator.go Adds new interface method for regex path validation
internal/controller/state/validation/validationfakes/fake_httpfields_validator.go Updates test fake with new regex validation method
internal/controller/state/graph/httproute_test.go Updates test expectations to accept regex paths
internal/controller/state/graph/httproute.go Implements regex path validation in route matching logic
internal/controller/state/dataplane/types.go Adds PathTypeRegularExpression constant
internal/controller/state/dataplane/convert_test.go Updates test to handle regex path conversion
internal/controller/state/dataplane/convert.go Adds regex path type conversion
internal/controller/nginx/config/validation/http_njs_match_test.go Removes obsolete path validation test
internal/controller/nginx/config/validation/http_njs_match.go Removes path validation method moved to common
internal/controller/nginx/config/validation/http_filters.go Adds new path validation methods
internal/controller/nginx/config/validation/common_test.go Adds comprehensive regex path validation tests
internal/controller/nginx/config/validation/common.go Implements regex path validation with RE2 constraints
internal/controller/nginx/config/servers_test.go Updates test expectations for new path formatting
internal/controller/nginx/config/servers.go Implements regex location generation and path handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-project-automation github-project-automation bot moved this from External Pull Requests to 🏗 In Progress in NGINX Gateway Fabric Sep 23, 2025
Copy link
Contributor Author

fabian4 commented Sep 23, 2025

https://github.com/nginx/nginx-gateway-fabric/actions/runs/17919926662/job/50987553809?pr=3874
https://github.com/nginx/nginx-gateway-fabric/actions/runs/17919926622/job/50987553763?pr=3874

BTW I can't figure why above test are failing since I can't get any thing like that when I run unit test and lint on my env.

It says the github.com/nginx/agent/v3@v3.3.1 package sum mismatch and the go lint location is empty. I don't think I have touched the related code, So still confused about the reason here.

Copy link
Collaborator

sjberman commented Sep 23, 2025 •
edited
Loading

@fabian4 Yeah I saw similar issues on my fork recently as well, not sure what's happening there. We'll investigate.
fabian4 reacted with heart emoji

@sindhushiv sindhushiv moved this from 🏗 In Progress to External Pull Requests in NGINX Gateway Fabric Sep 25, 2025
Copy link
Contributor

salonichf5 commented Sep 26, 2025 •
edited
Loading

@fabian I just had the first successful run for my fork test PR, so hopefully the pipeline issues should resolve for you now.

I'll get on to testing your PR once we can run the linter and tests in the pipeline.
fabian4 reacted with heart emoji 

Problem: As regex path matching is missing support for NGF.
Solution: Add support for regex path matching, only allow a full path rewrite/redirect after the regex path match.
Testing: Add additional unit test case for regex path match.
@fabian4 fabian4 force-pushed the add_regex_for_path_matching branch from 7f70c31 to 895c15c Compare September 29, 2025 11:13
Copy link
Contributor

Testing done on the branch

Error expected cases:

  1. Test for Path types allowed - PASSED
 - path:
 type: RegularExpressionInvalid
 value: /coffee-hello///
The HTTPRoute "coffee" is invalid: 
* spec.rules[1].matches[0].path.type: Unsupported value: "RegularExpressionInvalid": supported values: "Exact", "PathPrefix", "RegularExpression"
* <nil>: Invalid value: null: some validation rules were not checked because the object was invalid; correct the existing errors to complete validation
  1. Invalid Regular expression

Case 1: Drops its if starts with ^

 Last Transition Time: 2025年10月01日T21:21:34Z
 Message: Dropped Rule(s): spec.rules[1].matches[0].path.value: Invalid value: "^/foo$": must start with / and must not include any whitespace character, `{`, `}` or `;` (e.g. '/', or '/path', or '/path/subpath-123', regex used for validation is '/[^\s{};]*')
 - path:
 type: RegularExpression
 value: ^/foo$

Case 2: unescaped $


 - path:
 type: RegularExpression
 value: /foo$
ime: 2025年10月01日T21:22:59Z
 Message: Dropped Rule(s): spec.rules[1].matches[0].path.value: Invalid value: "/foo$": invalid unescaped `$` at position 4 in path '/foo$'

Case 3: Very long pattern

 - path:
 type: RegularExpression
 value: /aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
The HTTPRoute "coffee" is invalid: 
* spec.rules[1].matches[0].path.value: Too long: may not be more than 1024 bytes
* <nil>: Invalid value: null: some validation rules were not checked because the object was invalid; correct the existing errors to complete validation
  1. Malicious attacks using regex ( got these from malicious intent regex from chatgpt)
Regex Pattern Example Input Why Unsafe / Invalid Expected Outcome Actual Outcome
/(a+)+ /aaaaaa... (1M chars) Catastrophic backtracking (ReDoS) Reject Accepted
/(.+)+ /longpath.... Nested quantifiers cause blowup Reject Accepted
/.* /foo, /bar Overly broad, matches everything Reject Dropped Rule(s): spec.rules[1].matches[0].path.value: Invalid value: "/(?<=foo)bar": invalid RE2 regex ...
/(?<=foo)bar /foobar Lookbehind unsupported (RE2) Reject Dropped Rule(s): invalid RE2 regex, error parsing (?<=foo)bar
/1円 /foo Backreference, unsafe Reject Dropped Rule(s): invalid escape sequence 1円
/(a /a Unterminated group (syntax error) Reject Dropped Rule(s): missing closing )
/.{0,} /longinput Equivalent to match-all Reject Dropped Rule(s): must not include {, } or ;
/.../. /../../etc/passwd Path traversal pattern Reject Accepted

Copy link
Contributor

Testing done on the branch

Checking functionality that Regex works as expected

Regex Pattern Should Match Should NOT Match Results
/foo /foo, /foo/, /foo/bar, /foobar /bar, /fo PASS
/foo.*bar /foo123bar, /foo/bar /foo, /bar PASS
/api/v[0-9]+ /api/v1, /api/v12, /api/v123 /api/, /api/v PASS
/ /, /foo, // this matches everything should we allow it? PASS
`/(users admins)/[a-z]+` /users/john, /admins/alice PASS

return fmt.Sprintf("~ %s", rule.Path)
default:
return rule.Path
return ""// should never happen because path type is validated earlier
Copy link
Contributor

@salonichf5 salonichf5 Oct 1, 2025

Choose a reason for hiding this comment

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

should we panic here like we do for other cases @ciarams87 ?

Copy link
Contributor

@salonichf5 salonichf5 Oct 1, 2025

Choose a reason for hiding this comment

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

i guess its fine, this is how we have handled it in the past so should be good

Copy link
Contributor

@ciarams87 ciarams87 Oct 2, 2025

Choose a reason for hiding this comment

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

I think your first instinct was correct actually - this should be a panic. Now this function silently returns an invalid location path instead of at least returning the original path as a fallback. This is a programmer error (all path types should be handled), not a runtime error from user input (which is validated earlier), so panic() is appropriate and consistent.

salonichf5 reacted with thumbs up emoji
Copy link
Contributor

We need to also validate preexisting behaviour for URLRewrite, Redirect and other filters.

Copy link
Contributor

@fabian4 Thank you for your patience. We are currently testing Regex thoroughly—both on its own and in combination with our other filters.
We’ll consolidate all findings and provide you with a complete review of the changes required, so they can be addressed together.

I really appreciate your contribution. From my initial round of testing, everything looks good overall, with only a few exceptions that we’d like to discuss as a team to decide whether we want to allow them or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ciarams87 ciarams87 ciarams87 left review comments

@salonichf5 salonichf5 salonichf5 requested changes

Copilot code review Copilot Copilot left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Projects
Status: External Pull Requests
Milestone
v2.2.0
Development

Successfully merging this pull request may close these issues.

Regex for path matching: RegularExpression

AltStyle によって変換されたページ (->オリジナル) /