Skip to content

net/http: path values ​​are not passed to other *http.ServeMux #69077

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

Closed
covrom opened this issue Aug 27, 2024 · 9 comments
Closed

net/http: path values ​​are not passed to other *http.ServeMux #69077

covrom opened this issue Aug 27, 2024 · 9 comments

Comments

@covrom
Copy link

covrom commented Aug 27, 2024

Go version

go version go1.23.0 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/sdfg/.cache/go-build'
GOENV='/home/sdfg/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/sdfg/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/sdfg/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/sdfg/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4261393948=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I create a test with a main router and a subrouter and try to provide the path value from the main router into subrouter handler:

package main

import (
	"fmt"
	"io"
	"net/http"
	"net/http/httptest"
	"net/url"
	"testing"
)

func Test_main(t *testing.T) {
	rt1 := http.NewServeMux()
	rt2 := http.NewServeMux()

	rt1.HandleFunc("GET /{myid}/{tail...}", func(w http.ResponseWriter, r *http.Request) {
		t.Logf("r1.GET /{myid}/{tail...}: myid=%q tail=%q", r.PathValue("myid"), r.PathValue("tail"))

		p := stripToLastSlash(r.URL.Path, 2)
		rp := stripToLastSlash(r.URL.RawPath, 2)
		r2 := new(http.Request)
		*r2 = *r
		r2.URL = new(url.URL)
		*r2.URL = *r.URL
		r2.URL.Path = p
		r2.URL.RawPath = rp

		tail := r.PathValue("tail")
		if tail == "" {
			t.Error("r1: tail not found")
		}

		r2.SetPathValue("tail", tail)

		rt2.ServeHTTP(w, r2)
	})

	rt2.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) {
		t.Logf("r2.GET /: tail=%q", r.PathValue("tail"))
		if r.PathValue("tail") == "" {
			t.Error("r2: tail not found")
		}
		fmt.Fprintf(w, "%s", r.PathValue("tail"))
	})

	ts := httptest.NewServer(rt1)
	defer ts.Close()

	if _, body := testRequest(t, ts, "GET", "/123/456/789", nil); body != "456/789" {
		t.Error("tail was not provided")
	}
}

func stripToLastSlash(s string, cnt int) string {
	pos := 0
	for i, r := range s {
		if r == '/' {
			pos = i
			cnt--
			if cnt <= 0 {
				break
			}
		}
	}
	return s[pos:]
}

func testRequest(t *testing.T, ts *httptest.Server, method, path string, body io.Reader) (*http.Response, string) {
	req, err := http.NewRequest(method, ts.URL+path, body)
	if err != nil {
		t.Fatal(err)
		return nil, ""
	}

	resp, err := http.DefaultClient.Do(req)
	if err != nil {
		t.Fatal(err)
		return nil, ""
	}

	respBody, err := io.ReadAll(resp.Body)
	if err != nil {
		t.Fatal(err)
		return nil, ""
	}
	defer resp.Body.Close()

	return resp, string(respBody)
}

What did you see happen?

tail path value is not provided to subrouter:

main_test.go:17: r1.GET /{myid}/{tail...}: myid="123" tail="456/789"
main_test.go:39: r2.GET /: tail=""
main_test.go:41: r2: tail not found
main_test.go:50: tail was not provided

What did you expect to see?

r2.GET / must returns body "456/789"

@seankhliao
Copy link
Member

This comes from the way you've shallow cloned internal http.Request state, resulting in the SetPathValue being set to path matches which will be overridden by ServeMux.

Closing as not a bug.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Aug 27, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608655 mentions this issue: net/http: path values ​​are not passed to other *http.ServeMux

@covrom
Copy link
Author

covrom commented Aug 27, 2024

@seankhliao but no other way to do this! http.Request.Clone() do the same. Using new Request is too much.

@covrom
Copy link
Author

covrom commented Aug 27, 2024

It is not possible to simply create a router that would use path values from an upstream router that uses wildcards.
Request.SetPathValue sets the private "match" field if the value is present in the request wildcards.
Serving request in another *http.ServeMux with different URL (e.g. trimmed) clears the private "match" field and refills it with the URL.
http.Request.Clone() also copies the request's private "matches" field and not provide path values.
Using new Request to provide path values is too much. Using full URL path in all routers limits the flexibility of creating subrouters.
Request.SetPathValue should provide all manually set values ​​to another *http.ServeMux without clearing it.

@w3nl1ng
Copy link

w3nl1ng commented Aug 27, 2024

I think this can help you
you can debug and step into this line rt2.ServeHTTP(w, r2)
ServerHTTP may dispatch the r2 in this line h, r.Pattern, r.pat, r.matches = mux.findHandler(r)
ensure that you passed first param to rt2.HandleFunc correctly
check /src/net/http/routing_tree.go/#func (n *routingNode) matchPathmethod for more details

package main

import (
	"fmt"
	"io"
	"net/http"
	"net/http/httptest"
	"net/url"
	"testing"
)

func Test_main(t *testing.T) {
	rt1 := http.NewServeMux()
	rt2 := http.NewServeMux()

	rt1.HandleFunc("GET /{myid}/{tail...}", func(w http.ResponseWriter, r *http.Request) {
		t.Logf("r1.GET /{myid}/{tail...}: myid=%q tail=%q", r.PathValue("myid"), r.PathValue("tail"))

		p := stripToLastSlash(r.URL.Path, 2)
		rp := stripToLastSlash(r.URL.RawPath, 2)
		r2 := new(http.Request)
		*r2 = *r
		r2.URL = new(url.URL)
		*r2.URL = *r.URL
		r2.URL.Path = p
		r2.URL.RawPath = rp

		tail := r.PathValue("tail")
		if tail == "" {
			t.Error("r1: tail not found")
		}

		r2.SetPathValue("tail", tail)

		rt2.ServeHTTP(w, r2)
	})

	rt2.HandleFunc("GET /{tail...}", func(w http.ResponseWriter, r *http.Request) {
		t.Logf("r2.GET /: tail=%q", r.PathValue("tail"))
		if r.PathValue("tail") == "" {
			t.Error("r2: tail not found")
		}
		fmt.Fprintf(w, "%s", r.PathValue("tail"))
	})

	ts := httptest.NewServer(rt1)
	defer ts.Close()

	if _, body := testRequest(t, ts, "GET", "/123/456/789", nil); body != "456/789" {
		t.Error("tail was not provided")
	}
}

func stripToLastSlash(s string, cnt int) string {
	pos := 0
	for i, r := range s {
		if r == '/' {
			pos = i
			cnt--
			if cnt <= 0 {
				break
			}
		}
	}
	return s[pos:]
}

func testRequest(t *testing.T, ts *httptest.Server, method, path string, body io.Reader) (*http.Response, string) {
	req, err := http.NewRequest(method, ts.URL+path, body)
	if err != nil {
		t.Fatal(err)
		return nil, ""
	}

	resp, err := http.DefaultClient.Do(req)
	if err != nil {
		t.Fatal(err)
		return nil, ""
	}

	respBody, err := io.ReadAll(resp.Body)
	if err != nil {
		t.Fatal(err)
		return nil, ""
	}
	defer resp.Body.Close()

	return resp, string(respBody)
}
=== RUN   Test_main
    main_test.go:17: r1.GET /{myid}/{tail...}: myid="123" tail="456/789"
    main_test.go:39: r2.GET /: tail="456/789"
--- PASS: Test_main (0.00s)
PASS
ok      gofix   0.732s

@covrom
Copy link
Author

covrom commented Aug 27, 2024

@w3nl1ng Ok, that's fine for 'tail'. But what about 'myid'? What if we need to pass 'myid' without wildcard in rt2.HandleFunc?

func TestPathValueProviding(t *testing.T) {
	rt1 := http.NewServeMux()
	rt2 := http.NewServeMux()

	rt1.HandleFunc("GET /{myid}/{tail...}", func(w http.ResponseWriter, r *http.Request) {
		myid := r.PathValue("myid")
		if myid == "" {
			t.Error("r1: myid not found")
		}
		r2 := r.Clone(r.Context())

		r2.SetPathValue("myid", myid)

		r2.URL.Path = "/" + r.PathValue("tail")
		r2.URL.RawPath = ""

		rt2.ServeHTTP(w, r2)
	})

	rt2.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) {
		if r.PathValue("myid") == "" {
			t.Error("r2: myid not found")
		}
		t.Log(r.URL.Path)
	})

	ts := httptest.NewServer(rt1)
	defer ts.Close()

	res, err := http.Get(ts.URL + "/123/456/789")
	if err != nil {
		t.Fatal(err)
	}
	res.Body.Close()

	if res.StatusCode != 200 {
		t.Error("status code", res.StatusCode)
	}
}

@w3nl1ng
Copy link

w3nl1ng commented Aug 28, 2024

@covrom If you want get 'myid' in rt2, you should register it at url.
you can check the code in /src/net/http/routing_tree.go/#func (n *routingNode) matchPath method.
If you do not register wildcard in the url, the associated pattern will not be matched

	// Lastly, match the pattern (there can be at most one) that has a multi
	// wildcard in this position to the rest of the path.
	if c := n.multiChild; c != nil {
		// Don't record a match for a nameless wildcard (which arises from a
		// trailing slash in the pattern).
		if c.pattern.lastSegment().s != "" {
			matches = append(matches, pathUnescape(path[1:])) // remove initial slash
		}
		return c, matches
	}
	return nil, nil

@w3nl1ng
Copy link

w3nl1ng commented Aug 28, 2024

@covrom This means if you call HandleFunc like this, it won't match any pattern for you. The value pairs you previously set yourself will also be overwritten

rt2.HandleFunc("GET /", xxx)

And if you call it like this, it will try to set the value of 'a', 'b' and 'c' for you.

rt2.HandleFunc("GET /{a}{b}{c...}", xxx)

let's back to this line, what is important is that your r2 are not passed to rt2 unchanged, ServeHTTP will dispatch r2 according to rt2's internal information

rt2.ServeHTTP(w, r2)

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 a pull request may close this issue.

5 participants