-
Notifications
You must be signed in to change notification settings - Fork 3
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
feature: supports regex for origin #64
feature: supports regex for origin #64
Conversation
WalkthroughThe changes introduced in this update primarily focus on enhancing the CORS configuration by adding support for wildcard allowed origins. This is achieved by introducing a new field Changes
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- config.go (1 hunks)
- plugin.go (2 hunks)
Additional comments: 2
config.go (1)
- 13-20: The addition of the
AllowedOriginWildcard
field in theCORSConfig
struct is a good way to provide flexibility in specifying allowed origins using regular expressions. However, it's important to note that regular expressions can be complex and prone to errors if not used carefully. It would be beneficial to add a comment explaining what this field does and perhaps providing an example of how to use it. Also, consider adding validation to ensure that the provided regular expression is valid.+ // AllowedOriginWildcard: A regular expression to match allowed origins. If set, AllowedOrigin is ignored. + // Example: "^https?://(?:.+\.)?mydomain\.com$" AllowedOriginWildcard string `mapstructure:"allowed_origin_wildcard"`plugin.go (1)
- 3-9: The import section looks fine. The new import "regexp" is used for regular expression matching.
@cv65kr Hey hey 👋 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- plugin.go (3 hunks)
Additional comments: 6
plugin.go (6)
3-9: The import of the "regexp" package is added to support the new feature. This is a standard package in Go and is safe to use.
32-39: A new field
allowedOriginRegex
of type*regexp.Regexp
is added to thePlugin
struct. This is a good practice as it allows pre-compilation of the regex, improving performance.72-85: The
Init
function is updated to compile the regular expression from theAllowedOriginWildcard
field if it is set. TheAllowOriginFunc
option of thecors.Options
struct is set to use this regular expression for matching the origin. This is a good practice as it allows the use of a function to determine whether an origin is allowed, providing more flexibility. However, it's important to handle the error thatregexp.Compile
might throw. If the regular expression is invalid, the function will return an error, which should be handled appropriately to prevent a panic at runtime.* 76-81: + if p.cfg.CORS.AllowedOriginWildcard != "" { + var err error + p.allowedOriginRegex, err = regexp.Compile(p.cfg.CORS.AllowedOriginWildcard) + if err != nil { + return errors.E(op, err) + } + }
- 82-84: The
AllowOriginFunc
is set to a function that uses the compiled regular expression to match the origin. This is a good practice as it allows the use of a function to determine whether an origin is allowed, providing more flexibility.* 82-84: + opts.AllowOriginFunc = func(origin string) bool { + return p.allowedOriginRegex.MatchString(origin) + }
#64 (comment)
Will do, I was wait for your initial review to ensure that solution could be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- config.go (1 hunks)
- plugin.go (3 hunks)
Files skipped from review due to trivial changes (1)
- config.go
Additional comments: 3
plugin.go (3)
3-9: The import of the "regexp" package is added correctly.
32-39: The addition of the
allowedOriginRegex
field to thePlugin
struct is correct. It will hold the compiled regular expression for matching allowed origins.72-85: The logic for compiling the regular expression and setting the
AllowOriginFunc
is correct. However, it's important to handle the error thatregexp.Compile
might throw if the regular expression is invalid. This is done correctly here. If the regular expression is invalid, the function will return an error, preventing the application from starting with an invalid configuration.Please ensure that the changes are reflected in the documentation and that users are aware of the new configuration option and its implications.
@rustatian PR updated, documentation added - roadrunner-server/roadrunner-docs#156 😄 |
Cool, thank you very much @cv65kr 👍 |
Reason for This PR
closes: roadrunner-server/roadrunner#1709
Description of Changes
Added new configuration property
allowed_origin_wildcard
which allows to use regex in terms of CORS origin. Ifallowed_origin_wildcard
is set up thenallowed_origin
is ignored.License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]
git commit -s
).CHANGELOG.md
.Summary by CodeRabbit