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

URLParam sometimes returns URL-encoded variables #832

Open
cespedes opened this issue Jul 10, 2023 · 3 comments
Open

URLParam sometimes returns URL-encoded variables #832

cespedes opened this issue Jul 10, 2023 · 3 comments

Comments

@cespedes
Copy link

cespedes commented Jul 10, 2023

Hello!

As the title says, when I try to get a URL parameter from a http.Request object (calling chi.URLParam), it sometimes returns a URL-encoded result, and it only depends on whether r.URL.RawPath is set or not.

This is because in function *Mux.routeHTTP(), it checks if r.URL.RawPath is set and uses it instead of r.URL.Path; however, r.URL.RawPath is only present sometimes:

chi/mux.go

Lines 420 to 431 in 7f28096

routePath := rctx.RoutePath
if routePath == "" {
if r.URL.RawPath != "" {
routePath = r.URL.RawPath
} else {
routePath = r.URL.Path
}
if routePath == "" {
routePath = "/"
}
}

IMHO, the best course of action will be to use url.EscapedPath() to get the routePath and then url.PathUnescape() to set every value.

Simple program to show the bug:

package main

import (
        "fmt"
        "net/http"

        "github.com/go-chi/chi/v5"
)

func main() {
        r := chi.NewRouter()
        r.Get("/{key}", func(w http.ResponseWriter, r *http.Request) {
                key := chi.URLParam(r, "key")
                fmt.Printf("Path=%q RawPath=%q key=%q\n", r.URL.Path, r.URL.RawPath, key)
                fmt.Fprintf(w, "Path=%q RawPath=%q key=%q\n", r.URL.Path, r.URL.RawPath, key)
        })
        http.ListenAndServe(":3333", r)
}

Simple execution to see the problem:

$ curl "http://localhost:3333/It%20is%20great"
Path="/It is great" RawPath="" key="It is great"
$ curl "http://localhost:3333/It's%20great"
Path="/It's great" RawPath="/It's%20great" key="It's%20great"

If it is OK for you, I will open a Pull Request.

Thank you very much,

Juan

@prm-dan
Copy link

prm-dan commented Apr 9, 2024

+1. I'm hitting this same issue too. The logic seems inconsistent.

http://localhost:5150/v1/content/{contentID}

This URL

http://localhost:5150/v1/content/testid%1FABC

extracts contentId as

contentID=testid\u001fABC

But this URL

http://localhost:5150/v1/content/testid%3BABC

extracts contentId as

contentID=testid%3BABC

@brasic
Copy link

brasic commented Sep 27, 2024

We hit this bug. Because the data only sometimes needs unescaping depending on whatever inside go sets the URL.RawPath value in net/http, it's not safe to unconditionally apply url.PathUnescape(), since that would not correctly handle certain edge cases involving double-url-escaped strings, like http://example.com/my%2520data (with an escaped percent), which application logic should see as my%20data, but a naive approach would inappropriately yield my data. I'm using this unfortunate workaround:

func SafeURLParam(r *http.Request, key string) string {
	rctx := chi.RouteContext(r.Context())
	if rctx == nil {
		return ""
	}
	raw := rctx.URLParam(key)
	// Only apply unescaping if chi built the param map using the raw path.
	// Corresponds to the logic at
	// https://github.com/go-chi/chi/blob/v5.1.0/mux.go#L433-L437
	if r.URL.RawPath == "" {
		return raw
	}
	unescaped, err := url.PathUnescape(raw)
	if err != nil {
		slog.Warn("bad URL escape", "error", err, "param", raw)
		return ""
	}
	return unescaped
}

@BladeMcCool
Copy link

BladeMcCool commented Oct 8, 2024

same as issue 641 and issue 642 right? still no fix after 3 years? yeah i know ... i should submit a PR - well i just switched to using query params instead.

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

No branches or pull requests

4 participants