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

feat: support http-valid characters in ingress paths #821

Merged
merged 2 commits into from
Jan 20, 2024

Conversation

worstell
Copy link
Contributor

fixes #811

@github-actions github-actions bot changed the title support http-valid characters in ingress paths feat: support http-valid characters in ingress paths Jan 19, 2024
@worstell worstell force-pushed the worstell/20240119-http-parsing branch from 05c0a00 to 2c54469 Compare January 19, 2024 22:28
Copy link
Collaborator

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

I think a minor change is needed to the matching pattern, but otherwise LGTM.

@@ -79,7 +79,7 @@ type IngressPathComponent interface {
type IngressPathLiteral struct {
Pos Position `parser:"" protobuf:"1,optional"`

Text string `parser:"@Ident" protobuf:"2"`
Text string `parser:"@~Whitespace+" protobuf:"2"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this will need to be something like @~(Whitespace | '/' | '{' | '}')+, otherwise it will just keep consuming, including path parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah mixed up literal and the path parameter matchers. updated to @~(Whitespace | '/' | '{' | '}')+ for literal and leaving as '{' @Ident '}' for the path param - does that sound right?

backend/schema/parser.go Outdated Show resolved Hide resolved
go-runtime/compile/schema_test.go Outdated Show resolved Hide resolved
@worstell worstell force-pushed the worstell/20240119-http-parsing branch from 7f35bd6 to 8c7879a Compare January 20, 2024 01:45
@worstell worstell force-pushed the worstell/20240119-http-parsing branch from 96bc754 to 0a569d0 Compare January 20, 2024 01:53
@worstell worstell merged commit b7cbc0e into main Jan 20, 2024
@worstell worstell deleted the worstell/20240119-http-parsing branch January 20, 2024 01:59
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.

We need to be able to add support for both periods (.) and hyphens (-) in HTTP ingress paths.
2 participants