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

Non-default basePath causes issues with URL path escaped subject names #1525

Open
pkwarren opened this issue Nov 19, 2024 · 5 comments · May be fixed by #1526
Open

Non-default basePath causes issues with URL path escaped subject names #1525

pkwarren opened this issue Nov 19, 2024 · 5 comments · May be fixed by #1526
Labels
bug Something isn't working

Comments

@pkwarren
Copy link

Problem

Loading the schema registry UI or attempting to click 'Produce Record' on a topic integrated with the schema registry fails. The console shows 500 errors, and when looking at network requests there is a 200 request for the schema in the topic but 404 errors attempting to fetch the topic's schema references (if the schema references require URL path escaping - i.e. %2F for /).

Steps to reproduce

  • Configure a schema registry for a Protobuf topic (i.e. some-topic-value) which has one or more references with / in their names.
  • Configure the console with a valid schemaRegistry and a non-default server -> basePath.
  • Attempt to access 'Schema Registry' on the navigation on the left or attempt to produce a record to a schema registry enabled topic.

Suggested Fix

The problem appears to be related to the middleware which is responsible for stripping the basePath from incoming requests:

// Strip prefix from the request url
var path string
rctx := chi.RouteContext(r.Context())
if rctx.RoutePath != "" {
path = rctx.RoutePath
} else {
path = r.URL.Path
}

This section of code needs to use the r.URL.RawPath (if present) to build the rctx.RoutePath, not the r.URL.Path. Here's an example middleware from Chi: https://github.com/go-chi/chi/blob/6ceb498b221efa11f559602beb2463b8b52c2161/middleware/clean_path.go#L18-L22. It should probably also remove the prefix from RawPath as well for consistency.

I'm happy to raise a PR with the suggested fix.

@weeco
Copy link
Contributor

weeco commented Nov 19, 2024

Hey @pkwarren , thanks for the detailed report and your analysis. I'm wondering what Console version you are running?

In the past we already had issues with subjects that require URL encoding, but I'm certain that we resolved these for the schema registry pages. We haven't specifically tested with URL rewriting though. Whenever URL encoding is needed (like the slash) it should be replaced by %2F (as you already mentioned). Therefore, I think the middleware shouldn't find more slashes than usual?

@pkwarren
Copy link
Author

Hey @pkwarren , thanks for the detailed report and your analysis. I'm wondering what Console version you are running?

Seeing this with v2.7.2 - I also checked latest master and it doesn't appear to have a fix.

In the past we already had issues with subjects that require URL encoding, but I'm certain that we resolved these for the schema registry pages. We haven't specifically tested with URL rewriting though. Whenever URL encoding is needed (like the slash) it should be replaced by %2F (as you already mentioned). Therefore, I think the middleware shouldn't find more slashes than usual?

Yes we are only able to reproduce this with URL rewriting. With the default basePath, everything works great.

@weeco
Copy link
Contributor

weeco commented Nov 19, 2024

Could you check the network request coming from the frontend and whether the subject is actually requested in an encoded form (i.e. slashes in subjectname encoded)?

@weeco weeco added the bug Something isn't working label Nov 19, 2024
@pkwarren
Copy link
Author

Could you check the network request coming from the frontend and whether the subject is actually requested in an encoded form (i.e. slashes in subjectname encoded)?

Yes it is - the subject in network inspector is path encoded. I've also reproduced from the command line with curl.

@weeco
Copy link
Contributor

weeco commented Nov 19, 2024

Thanks for the info. Maybe you are right with the middleware - the URL path may be decoded too early and slashes will be removed there. If you could come up with a PR that would be great

pkwarren added a commit to pkwarren/console that referenced this issue Nov 19, 2024
Update basePathMiddleware to use `RawPath` (if present) over `Path` to
ensure that URL path encoded subjects are preserved when routing
requests.

Update the middleware to also remove the prefix from `RawPath` for
consistency.

Fixes redpanda-data#1525.
@pkwarren pkwarren linked a pull request Nov 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants