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

httplibp2p: cannot mount a protocol at the root #2545

Closed
Tracked by #2523
aschmahmann opened this issue Aug 31, 2023 · 3 comments · Fixed by #2552
Closed
Tracked by #2523

httplibp2p: cannot mount a protocol at the root #2545

aschmahmann opened this issue Aug 31, 2023 · 3 comments · Fixed by #2552

Comments

@aschmahmann
Copy link
Collaborator

It seems from the spec PR that it should be possible to mount a protocol at the root (i.e. /).

However, from what I can tell though this will cause problems in go-libp2p because the / will be sliced off the prefix

h.ServeMux.Handle(path, http.StripPrefix(path, handler))
which will result in some infinite looping if you follow redirects (e.g. curl -L ) https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/net/http/server.go;l=2475-2479 since path != r.URL.Path will always be the case due to slicing the / off the front.

@sukunrt
Copy link
Member

sukunrt commented Sep 1, 2023

Can you elaborate here? I'm not sure if this is a problem, but my understanding of the http work is not great.

I'm using this example host and making requests via curl:

func Test_ExampleHost_RootPath(t *testing.T) {
	// Create the server
	server := libp2phttp.Host{
		InsecureAllowHTTP: true, // For our example, we'll allow insecure HTTP
		ListenAddrs:       []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/50221/http")},
	}

	server.SetHTTPHandlerAtPath("/hello/1", "/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		fmt.Println(r.URL.Path)
		w.Header().Add("Content-Type", "text/plain")
		w.Write([]byte("Hello World"))
	}))
	
	server.Serve()
}

This works fine with curl:
curl -Lv http://127.0.0.1:50221/ curl -Lv http://127.0.0.1:50221

This also works with:
curl -Lv --path-as-is "http://127.0.0.1:50221/abc/../"

using libp2phost.Host.NewConstrainedRoundTripper also works fine for me.

In https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/net/http/server.go;l=2475-2479
both path and r.URL.Path are the same unless r.URL.Path has . and .. elements like the example above(/abc/../).

@aschmahmann
Copy link
Collaborator Author

both path and r.URL.Path are the same unless r.URL.Path has . and .. elements like the example above(/abc/../).

They are also not the same if r.URL.Path does not start with a / which is what I'm running into here.


Maybe I'm misunderstanding how the composition is supposed to work here, but my understanding is that the httplibp2p.Host is supposed to operate roughly like a ServeMux itself.

Thanks for your test example, it gives something to write against. Here's a similar example demonstrating an infinite loop issue that comes up with nested ServeMux's.

Using standard HTTP ServeMux:

func Test_StandardHTTPNestedMux(t *testing.T) {
	rootMx := http.NewServeMux()
	nestedMx := http.NewServeMux()
	nestedMx.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		fmt.Println(r.URL.Path)
		w.Header().Add("Content-Type", "text/plain")
		w.Write([]byte("Hello World"))
	})
	rootMx.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		nestedMx.ServeHTTP(w, r)
	})

	l, err := net.Listen("tcp", "127.0.0.1:50221")
	if err != nil {
		t.Fatal(err)
	}
	http.Serve(l, rootMx)
}
❯ curl 127.0.0.1:50221/abc/def
Hello World

Using an httplibp2p host as the root:

func Test_HTTPLibp2pNestedMux(t *testing.T) {
	// Create the server
	server := libp2phttp.Host{
		InsecureAllowHTTP: true, // For our example, we'll allow insecure HTTP
		ListenAddrs:       []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/50221/http")},
	}

	nestedMux := http.NewServeMux()
	nestedMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		fmt.Println(r.URL.Path)
		w.Header().Add("Content-Type", "text/plain")
		w.Write([]byte("Hello World"))
	})

	server.SetHTTPHandlerAtPath("/hello/1", "/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		nestedMux.ServeHTTP(w, r)
	}))

	server.Serve()
}
❯ curl 127.0.0.1:50221/abc/def
<a href="/abc/def">Moved Permanently</a>.

❯ curl -L 127.0.0.1:50221/abc/def
curl: (47) Maximum (50) redirects followed

@sukunrt
Copy link
Member

sukunrt commented Sep 3, 2023

Thanks! This clarified the issue. There is a bug in the happy case too here. See the PR.

@p-shahi p-shahi mentioned this issue Sep 15, 2023
25 tasks
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.

2 participants