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

introduce MergePathStrategy for #521 #519 #523

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

emicklei
Copy link
Owner

No description provided.

@SVilgelm
Copy link
Contributor

SVilgelm commented Mar 2, 2023

I need this as soon as possible :) Please merge and release new version

route_builder.go Outdated
// MergePathStrategy is the active strategy for merging a Route path when building the routing of all WebServices.
// The value is set to TrimSlashStrategy
// PathJoinStrategy is an alternative strategy that is more strict [Security - PRISMA-2022-0227]
MergePathStrategy = TrimSlashStrategy
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to set it to PathJoinStrategy as default

Copy link
Owner Author

Choose a reason for hiding this comment

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

yes, agree. Reason for it is because that will fix the security issue that motivated the change (which turned out to be a breaking one).

@emicklei emicklei marked this pull request as ready for review March 5, 2023 21:50
README.md Outdated
@@ -96,6 +96,10 @@ There are several hooks to customize the behavior of the go-restful package.
- Compression
- Encoders for other serializers
- Use [jsoniter](https://github.com/json-iterator/go) by building this package using a build tag, e.g. `go build -tags=jsoniter .`
- Use the variable `MergePathStrategy` to change the behaviour of composing the Route path given a root path and a local route path
- versions >= 3.10.1 has set the value to `PathJoinStrategy` that fixes a reported security issue but may cause your services not to work correctly anymore.
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please add a link to the issue that was fixed?

Copy link
Contributor

@SVilgelm SVilgelm left a comment

Choose a reason for hiding this comment

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

LGTM

but I still don't understand how this PR #520 is related to that issue

@emicklei emicklei merged commit de058a5 into v3 Mar 9, 2023
brandond added a commit to brandond/k3s that referenced this pull request May 24, 2023
Fix regression in legacy API prefix, until upstream pulls in support for MergePathStrategy from emicklei/go-restful#523

Signed-off-by: Brad Davidson <[email protected]>
brandond added a commit to brandond/k3s that referenced this pull request May 24, 2023
Fix regression in legacy API prefix, until upstream pulls in support for MergePathStrategy from emicklei/go-restful#523

Signed-off-by: Brad Davidson <[email protected]>
brandond added a commit to k3s-io/k3s that referenced this pull request May 24, 2023
Fix regression in legacy API prefix, until upstream pulls in support for MergePathStrategy from emicklei/go-restful#523

Signed-off-by: Brad Davidson <[email protected]>
brandond added a commit to k3s-io/k3s that referenced this pull request May 24, 2023
Fix regression in legacy API prefix, until upstream pulls in support for MergePathStrategy from emicklei/go-restful#523

Signed-off-by: Brad Davidson <[email protected]>
emicklei added a commit that referenced this pull request Aug 5, 2023
* allow multiple samples for Write, issue #514

* update changelog

* chore: example handling request parameters with httpin (#518)

* use path package to join slash fragments #519 (#520)

* update hist

* update example openapi to use 3.10.1

* Add test for client request with and without trailing slash. (#522)

* Add test for client request with and without trailing slash.

* Correction.

* introduce MergePathStrategy

* Revert "introduce MergePathStrategy"

This reverts commit 709cf80.

* introduce MergePathStrategy for #521 #519 (#523)

* introduce MergePathStrategy for #521 #519

* update readme, set default to new strategy, add extra test

* link to security issue

* update change hist

* add hello world with TrimSlashStrategy

* two route example

* examples to show differences #519

* more route examples #519

* add examples for issue519 with path in root

* remove obsolete swagger example

* Update README.md

remover swagger12 mention

* allow multiple samples for Write, issue #514

---------

Co-authored-by: Ggicci <[email protected]>
Co-authored-by: Gerrit <[email protected]>
@emicklei emicklei deleted the feature/add-MergePathStrategy branch August 13, 2023 06:04
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.

2 participants