feat: Implement HTTP spec #2438

MarcoPolo · 2023-07-20T17:58:36Z

A MVP for libp2p HTTP spec libp2p/specs#508.

This adds an Host for doing request-response with HTTP semantics. The libp2phttp.Host contrasts with a "stream host" (basichost or host.Host interface). It is capable of doing HTTP over an HTTP transport and/or a stream transport (provided by the stream host).

What works in this MVP:

.well-known/libp2p
A helper func to give you the best roundtripper given a list of multiaddrs and an optional stream host.
A way of listening for HTTP on stream transports as well as a native HTTP transport.

My thoughts after implementing the spec is that the spec is a nice minimal set of constraints that are relatively easy to implement. Even making a compliant node using the the stock HTTP library is fairly straightforward.

If this looks good after a review, I would suggest merging this as an experimental feature to allow folks to start using it and getting feedback. I may try to integrate this into the IPNI stack to see how it feels.

Note: This branch targets marco/gostream which pulls in https://github.com/libp2p/go-libp2p-gostream into the monorepo.

Update Aug 2:
I've removed the libp2p-noise auth from the http spec, and replaced it with this spec focused only on auth: libp2p/specs#564.

I think for the mvp we can merge this without doing any peer id auth, since peer id auth is optional anyways.

BigLep · 2023-07-24T14:17:58Z

@MarcoPolo : for the "libp2p and HTTP" effort, what will the testing story be? I'm not saying that all needs to be in this PR, but I'm curious about where we're aiming to end up.

MarcoPolo · 2023-07-24T18:12:48Z

@MarcoPolo : for the "libp2p and HTTP" effort, what will the testing story be? I'm not saying that all needs to be in this PR, but I'm curious about where we're aiming to end up.

This PR adds basic tests that test end-to-end communication on top of stream transports or an HTTP transport. It tests that a stock HTTP client/server can interoperate. It also adds tests around peer id authentication. It doesn't currently, but will include a test that tests the SNI+TLS flow for peer id authentication, and tests that incorrect auth steps correctly fail (these are noted in before merge list in the PR body).

Once we have multiple implementations of the spec, we will leverage the multidim interop tester to test this as well. It should just work, with no changes needed to the test runner.

Was there a test you had in mind that was missing here? Or something that you feel should be highlighted?

BigLep · 2023-07-25T05:03:12Z

Was there a test you had in mind that was missing here? Or something that you feel should be highlighted?

Thanks for explaining. I didn't have anything else in mind. I had missed the tests you described when I took a quick scan earlier. My bad for not catching those.

Concerning multidimen interop running, we'll also need that for testing version compatibility as well right (even if go-libp2p is the only implementation to support this functionality for awhile)?

MarcoPolo · 2023-08-02T21:59:36Z

I've removed the libp2p-noise auth from the http spec, and replaced it with this spec focused only on auth: libp2p/specs#564. I've also removed auth from this PR.

I think for the mvp we can merge this without doing any peer id auth, since peer id auth is optional anyways.

gammazero

This looks great. I plan on using it soon.
Left some trivial suggestions.

p2p/http/libp2phttp.go

p2p/http/ping.go

p2p/http/libp2phttp.go

p2p/http/ping.go

p2p/http/libp2phttp.go

MarcoPolo · 2023-08-04T01:53:09Z

@gammazero I have a surprise for you: ipni/go-libipni#102

p2p/http/libp2phttp.go

p2p/security/noise/pb/payload.proto

MarcoPolo · 2023-08-08T22:38:58Z

p2p/http/libp2phttp.go

+	// lets us know if we've recently connected to an HTTP endpoint and might
+	// have a warm idle connection for it (managed by the underlying HTTP
+	// roundtripper). In some cases, this lets us reuse our existing custom roundtripper (i.e. SNI != host).
+	recentHTTPAddrs *lru.Cache[peer.ID, httpAddr]


This should probably be keyed by the addr+scheme+sni combo.

marten-seemann

Lots of my comments come from looking at the GoDoc (godoc -http=:6060), and putting myself into the shoes of a user of this package.

Should we split up libp2phttp.go into two files, one for the client and one for the server?

The following code doesn't work:

host, err := libp2phttp.New(libp2phttp.ListenAddrs([]ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/8080/http")}))
if err != nil {
	log.Fatal(err)
}
host.SetHttpHandler("/website", http.FileServer(http.Dir("/tmp")))
log.Fatal(host.Serve())

Haven't debugged it yet. It creates the well-known handler, but http://localhost:8080/website/ returns a 404.

p2p/security/noise/pb/payload.proto

p2p/http/ping.go

p2p/http/libp2phttp.go

marten-seemann · 2023-08-14T04:28:03Z

p2p/http/options.go

+	ServerMustAuthenticatePeerID bool
+}
+
+func RoundTripperPreferHTTPTransport(o roundTripperOpts) roundTripperOpts {


Missing documentation.

p2p/http/options.go

p2p/http/libp2phttp.go

marten-seemann · 2023-08-14T05:23:50Z

p2p/http/libp2phttp.go

+	ma.ForEach(addr, func(c ma.Component) bool {
+		switch c.Protocol().Code {
+		case ma.P_IP4, ma.P_IP6, ma.P_DNS, ma.P_DNS4, ma.P_DNS6:
+			out.host = c.Value()
+		case ma.P_TCP, ma.P_UDP:
+			out.port = c.Value()
+		case ma.P_TLS:
+			out.useHTTPS = true
+		case ma.P_SNI:
+			out.sni = c.Value()
+
+		}
+		return out.host == "" || out.port == "" || !out.useHTTPS || out.sni == ""
+	})


Please don't reimplement multiaddr parsing. manet.ToNetAddr should provide half of what you need here.

That's what I did originally, but it's not actually what I want here. manet.ToNetAddr resolves the address, which I don't want to do. And it fails if there's an {/http,/tls/http, /https, /tls/sni/.../http} component in the multiaddr, which is expected in these cases.

But I think I agree with the underlying assumption that this should be easier to describe than a foreach and switch (which doesn't even check ordering). Maybe extend multiformats/go-multiaddr-fmt to include doing something like capture groups? Something like this regex:

/(?<host>ip4|ip6|dns|dns4|dns6)/(?<port>tcp|udp)...

p2p/http/libp2phttp.go

MarcoPolo · 2023-08-15T04:02:26Z

Lots of my comments come from looking at the GoDoc (godoc -http=:6060), and putting myself into the shoes of a user of this package.

Nice! Thanks!

The following code doesn't work:

host, err := libp2phttp.New(libp2phttp.ListenAddrs([]ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/8080/http")}))
if err != nil {
	log.Fatal(err)
}
host.SetHttpHandler("/website", http.FileServer(http.Dir("/tmp")))
log.Fatal(host.Serve())

When you go to "/website" the fileserver is seeing the full path "/website" and you get a 404 because you don't have a /tmp/website folder. This is the exact same behavior you get with a servemux (m.Handle("/website/", http.FileServer(http.Dir("/tmp")))).

You should strip the prefix of the path:

host.SetHttpHandler("/website", http.StripPrefix("/website", http.FileServer(http.Dir("/tmp"))))

Maybe this should be the default behavior for SetHttpHandler?

MarcoPolo · 2023-08-16T19:55:19Z

I've changed the behavior of SetHTTPHandler to strip the prefix for the handler automatically. I can't think of use cases where you wouldn't want that. You can still avoid it by setting a handler on the underlying servemux directly.

p2p/http/ping/ping.go

p2p/http/libp2phttp.go

gammazero

I am currently using this here: ipni/go-libipni#113

This looks great and I cannot wait until it gets merged.

p2p/http/libp2phttp.go

marten-seemann · 2023-08-23T04:39:40Z

p2p/http/libp2phttp.go

+	// ServeInsecureHTTP indicates if the server should serve unencrypted HTTP requests over TCP.
+	ServeInsecureHTTP bool
+	// ServeMux is the http.ServeMux used by the server to serve requests
+	ServeMux http.ServeMux


If not set, http.DefaultServeMux is used? Not sure if that's how it's implemented right now, but that's how http.Server works.

No, it will not use http.DefaultServeMux. To me, that's a default for the standard library, and I think it would be confusing if we referenced it here. Users would create a new host that automatically exposes things they weren't expecting.

The reason this is public is to allow a user to set a custom servemux, or to manually manage the mapping outside of SetHTTPHandler. I'll expand the comment to mention this.

This should probably be a pointer if we want to let users set a custom servemux

marten-seemann · 2023-08-23T04:57:00Z

p2p/http/libp2phttp.go

+// - integrate with the conn gater and resource manager
+
+// ProtocolMeta is metadata about a protocol.
+type ProtocolMeta struct {


Not sure I like Meta. Alternative: ProtocolConfig?

Config implies that this is a configurable, but this is metadata about a peer's supported protocols.

marten-seemann · 2023-08-23T04:57:06Z

p2p/http/libp2phttp.go

+	Path string `json:"path"`
+}
+
+type PeerMeta map[protocol.ID]ProtocolMeta


PeerProtocols?

No strong preference here, but I do prefer that PeerMeta is consistent with ProtocolMeta.

marten-seemann · 2023-08-23T05:00:11Z

p2p/http/libp2phttp.go

+}
+
+// GetPeerMetadata gets a peer's cached protocol metadata from the http host.
+func (h *Host) GetPeerMetadata(server peer.ID) (PeerMeta, bool) {


Is there any reason to return the whole map here? Alternative: SupportsProtocol(peer.ID) (path string, bool). It would be nice if we didn't have to make the map an exported type.

The reason to export the whole type is to give users flexibility in managing the Host's peerMetadata cache without exposing that cache. This is useful if they want to add out of band knowledge of a peer's metadata. This method along with Set and Remove should let users do anything they need to with this metadata cache.

p2p/http/libp2phttp.go

marten-seemann · 2023-08-23T05:08:21Z

p2p/http/libp2phttp.go

+// NewRoundTripper returns an http.RoundTripper that can fulfill and HTTP
+// request to the given server. It may use an HTTP transport or a stream based
+// transport. It is valid to pass an empty server.ID.
+func (h *Host) NewRoundTripper(server peer.AddrInfo, opts ...RoundTripperOption) (http.RoundTripper, error) {


I'm struggling with this abstraction. An http.RoundTripper is supposed to be able to issue requests to any HTTP host. This is more than just a theoretical concern: The http.Client has a Transport field that takes an http.RoundTripper, and would then be able to follow redirects. If we return a RoundTripper here that's scoped a single host, it won't be possible to redirect from one libp2p+HTTP host to another.

I don't have a good solution for this now, I'll have to think about the correct abstraction more.

it won't be possible to redirect from one libp2p+HTTP host to another.

I agree this is an interesting use case we should try to support.

The thing I like about this api is that a user can pass in many multiaddrs and the best transport is chosen for them. There isn't anywhere else we could do this besides when we create a round tripper. But, thinking more about this, this doesn't have to be a property of the core round tripper. We can have a round tripper on top of some core round tripper that can do this logic. It would be great if the core round tripper could, as you say, fulfill any HTTP request regardless of the transport.

Here's one idea how to do this:

Change this function to accept no parameters. It just returns an http.RoundTripper.

This roundtripper can fulfill HTTP requests to the following URIs:
a. http:// and https://, just like the http.DefaultTransport does.
b. multiaddr:<peer-multiaddr>. Which would be a new URI scheme for multiaddrs. The value after multiaddr: is simply the string encoded version of the multiaddr. The multiaddr can include a peer's transport information, or could include only the peer id /p2p/<peer-id> and rely on discovery or out-of-band knowledge of a peer's addresses. Note this multiaddr can also be an http transport multiaddr.

This would work but would lead to us having to define the httppath component since we'll need a way to reference the HTTP path we want to use in the multiaddr.

We would probably also want to register the multiaddr uri scheme in IANA (but could defer).

Here's a branch of this idea: https://github.com/libp2p/go-libp2p/compare/marco/http...marco/newroundtripper?expand=1. It includes an httppath implementation, but we should standardize that (not use the private area) if we go that route. It also has a test demonstrating the redirect to other host (using multiaddrs).

For now let’s remove the NewRoundtripper and only expose the NamespacedRoundtripper and NamespacedClient. We can then add the NewRoundTripper later.

The only thing we loose is redirects across hosts. I thinks that’s fine to omit for this initial version. And we have plan above on how to add it.

I've renamed this to NewConstrainedRoundTripper, I think that will unblock us here. In a future version we can introduce the NewRoundTripper that accepts a multiaddr uri and can handle redirects across hosts.

And don't use the http.DefaultRoundTripper and cast

Covered by #2511

Co-authored-by: Andrew Gillis <[email protected]>

marten-seemann

Ship it!

MarcoPolo force-pushed the marco/http branch from d0f6eb5 to 6f0fa81 Compare July 20, 2023 17:59

This was referenced Jul 27, 2023

Add initial prototype of http over libp2p #2218

Closed

http: libp2p + http #2034

Closed

MarcoPolo requested a review from marten-seemann August 2, 2023 22:28

gammazero reviewed Aug 3, 2023

View reviewed changes

p2p/http/libp2phttp.go Outdated Show resolved Hide resolved

p2p/http/ping.go Outdated Show resolved Hide resolved

p2p/http/libp2phttp.go Outdated Show resolved Hide resolved

p2p/http/ping.go Outdated Show resolved Hide resolved

p2p/http/libp2phttp.go Outdated Show resolved Hide resolved

aschmahmann mentioned this pull request Aug 3, 2023

Serve Trustless IPFS HTTP Gateway API via libp2p (experimental) ipfs/kubo#10049

Closed

MarcoPolo mentioned this pull request Aug 4, 2023

libp2p + HTTP an exploratory refactor ipni/go-libipni#102

Closed

MarcoPolo commented Aug 8, 2023

View reviewed changes

p2p/http/libp2phttp.go Outdated Show resolved Hide resolved

MarcoPolo commented Aug 8, 2023

View reviewed changes

p2p/security/noise/pb/payload.proto Outdated Show resolved Hide resolved

MarcoPolo commented Aug 8, 2023

View reviewed changes

MarcoPolo mentioned this pull request Aug 11, 2023

go-libp2p v0.30 #2428

Closed

21 tasks

marten-seemann reviewed Aug 14, 2023

View reviewed changes

marten-seemann mentioned this pull request Aug 15, 2023

go-libp2p v0.31 #2481

Closed

23 tasks

Wondertan reviewed Aug 16, 2023

View reviewed changes

p2p/http/ping/ping.go Outdated Show resolved Hide resolved

p2p/http/libp2phttp.go Outdated Show resolved Hide resolved

MarcoPolo requested a review from marten-seemann August 18, 2023 03:50

gammazero reviewed Aug 19, 2023

View reviewed changes

p2p/http/libp2phttp.go Outdated Show resolved Hide resolved

gammazero approved these changes Aug 19, 2023

View reviewed changes

MarcoPolo commented Aug 21, 2023

View reviewed changes

p2p/http/libp2phttp.go Outdated Show resolved Hide resolved

marten-seemann reviewed Aug 23, 2023

View reviewed changes

MarcoPolo mentioned this pull request Aug 23, 2023

libp2phttp: Integrate with the connection gater and resource manager #2511

Open

MarcoPolo force-pushed the marco/http branch from 5d85daa to 912f92d Compare August 24, 2023 00:20

MarcoPolo mentioned this pull request Aug 24, 2023

feat: Bring in gostream to the monorepo #2521

Closed

MarcoPolo requested a review from marten-seemann August 24, 2023 18:50

MarcoPolo and others added 25 commits August 28, 2023 21:46

Add well known handler example

eea1710

Handle empty path

e756bad

Fix typo

b8c1380

Make RoundTripperOption public so users can allocate a slice of options

647653d

Rename HTTPHost to Host

1f12a58

Make the host.WellKnownHandler public

c31efcd

Make Add merge PeerMetadata. Introduce SetPeerMetadata

ac47208

Rename AddProtocolMapping to AddProtocolMeta. Expand comment

403b282

Expand comment on DefaultClientRoundTripper

8b9e11d

And don't use the http.DefaultRoundTripper and cast

Remove todo

eb9aa17

Covered by #2511

Fix comment typo

791e0f3

Fix comment typo

e53fa35

Create helper init fn

419660b

Rename NewRoundTripper to NewConstrainedRoundTripper

80fbb7d

Use pointer for Host.ServeMux

8c18fe4

Don't ignore err

c65d655

json decode from reader

34c5818

Nits

a4de205

Move setupListeners to method

a6176a7

Add comment for streamReadCloser

fcd5c05

Add more comments

39adff6

Add todo

5f75f4e

Defer connect until the round trip

9bca68a

Rebase gostream

1ecf648

Update p2p/http/libp2phttp.go

04ec0c0

Co-authored-by: Andrew Gillis <[email protected]>

MarcoPolo force-pushed the marco/http branch from 611e1a5 to 04ec0c0 Compare August 29, 2023 04:46

marten-seemann approved these changes Aug 29, 2023

View reviewed changes

MarcoPolo changed the base branch from marco/gostream to master August 29, 2023 05:51

MarcoPolo merged commit ac038db into master Aug 29, 2023
13 checks passed

MarcoPolo deleted the marco/http branch August 29, 2023 05:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement HTTP spec #2438

feat: Implement HTTP spec #2438

MarcoPolo commented Jul 20, 2023 •

edited

Loading

BigLep commented Jul 24, 2023

MarcoPolo commented Jul 24, 2023

BigLep commented Jul 25, 2023

MarcoPolo commented Aug 2, 2023

gammazero left a comment

MarcoPolo commented Aug 4, 2023

MarcoPolo Aug 8, 2023

marten-seemann left a comment

marten-seemann Aug 14, 2023

marten-seemann Aug 14, 2023

MarcoPolo Aug 16, 2023

MarcoPolo commented Aug 15, 2023

MarcoPolo commented Aug 16, 2023

gammazero left a comment

marten-seemann Aug 23, 2023

MarcoPolo Aug 23, 2023 •

edited

Loading

MarcoPolo Aug 25, 2023

marten-seemann Aug 23, 2023

MarcoPolo Aug 23, 2023

marten-seemann Aug 23, 2023

MarcoPolo Aug 23, 2023

marten-seemann Aug 23, 2023

MarcoPolo Aug 23, 2023

marten-seemann Aug 23, 2023

MarcoPolo Aug 24, 2023

MarcoPolo Aug 24, 2023

MarcoPolo Aug 24, 2023

marten-seemann left a comment

feat: Implement HTTP spec #2438

feat: Implement HTTP spec #2438

Conversation

MarcoPolo commented Jul 20, 2023 • edited Loading

BigLep commented Jul 24, 2023

MarcoPolo commented Jul 24, 2023

BigLep commented Jul 25, 2023

MarcoPolo commented Aug 2, 2023

gammazero left a comment

Choose a reason for hiding this comment

MarcoPolo commented Aug 4, 2023

Choose a reason for hiding this comment

marten-seemann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcoPolo commented Aug 15, 2023

MarcoPolo commented Aug 16, 2023

gammazero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcoPolo Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marten-seemann left a comment

Choose a reason for hiding this comment

MarcoPolo commented Jul 20, 2023 •

edited

Loading

MarcoPolo Aug 23, 2023 •

edited

Loading