You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "shreemaan-abhishek (via GitHub)" <gi...@apache.org> on 2024/11/28 09:04:36 UTC
[PR] fix: allow special characters in uri params [apisix]
shreemaan-abhishek opened a new pull request, #11788:
URL: https://github.com/apache/apisix/pull/11788
### Description
The underlying `lua-resty-radixtree` library used a regex that would cause route matching to fail when there are special characters in the uri parameters. This has been fixed in the radixtree router library, so we upgrade the version and add a test case that ensures special characters are allowed in uri parameters when using `radixtree_uri_with_parameter`.
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: allow special characters in uri params [apisix]
Posted by "mikyll (via GitHub)" <gi...@apache.org>.
mikyll commented on PR #11788:
URL: https://github.com/apache/apisix/pull/11788#issuecomment-2505712426
I tested this change and it doesn't seem to work for `%2F` (encoded `/`)...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: allow special characters in uri params [apisix]
Posted by "mikyll (via GitHub)" <gi...@apache.org>.
mikyll commented on PR #11788:
URL: https://github.com/apache/apisix/pull/11788#issuecomment-2517766817
@shreemaan-abhishek thank you for the reply, but I think you misunderstand my previous comment... I **agree** that `/` should only be treated as a **path separator**, it wouldn't make sense otherwise and it would also be a security issue.
**_However_**, that's exactly the reason why we need URL-encoding: when we URL-encode ("[percent-encoding](https://developer.mozilla.org/en-US/docs/Glossary/Percent-encoding)") characters, they are treated as **raw data**.
Currently, APISIX with `radixtree_uri_with_parameter` **doesn't fully support this**, since the router fails to route requests that contain `%2F` in a path parameter.
This PR fixed the problem for many special characters but **not** for `%2F` (encoded `/`)!
See the [comparison table](https://github.com/apache/apisix/pull/11788#issuecomment-2505712426) above.
Please see [issue #11810](https://github.com/apache/apisix/issues/11810) for further information.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: allow special characters in uri params [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek merged PR #11788:
URL: https://github.com/apache/apisix/pull/11788
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: allow special characters in uri params [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on PR #11788:
URL: https://github.com/apache/apisix/pull/11788#issuecomment-2507068275
@mikyll, this is expected behaviour as `/` is a delimiter for urls. If `/` is allowed as a special character, there will be no regularity in the boundary of the url parameter. Consider the following example:
route uri: `/v1/:id/products/:type/list`
request uri: `/v1/123/products/footwear/list`
If `/` is also accepted as a special character then the `id` url parameter would be incorrectly evaluated to `123/products/footwear/list`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: allow special characters in uri params [apisix]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on PR #11788:
URL: https://github.com/apache/apisix/pull/11788#issuecomment-2505644658
@shreemaan-abhishek lint issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org