http-transport legs #23
Conversation
|
relevant perhaps to @warpfork |
|
This now has a publisher, subscriber and a test to demonstrate syncing. |
| "github.com/multiformats/go-multiaddr" | ||
| ) | ||
|
|
||
| var defaultPollTime = time.Hour |
There was a problem hiding this comment.
I can imagine testing wanting to change this
| func (h *httpSubscriber) fetchHead(ctx context.Context) (cid.Cid, error) { | ||
| var cidStr string | ||
| if err := h.fetch(ctx, "head", func(msg io.Reader) error { | ||
| return json.NewDecoder(msg).Decode(&cidStr) |
There was a problem hiding this comment.
Beware that you might have fallen prey to golang/go#36225 here. You should either ensure there isn't a second JSON value, or use json.Unmarshal, which does that for you.
There was a problem hiding this comment.
the worry is there's extra tailing error from the server after a valid CID, and we should error rather than reading the first CID and moving on?
It feels like an edge case where i'd rather not write the longer code of reading into a buffer and then unmarshaling since the end result here is we're trusting the server to tell us the head, and if we don't trust that response there's an availability attack anyway.
There was a problem hiding this comment.
@willscott I think the issue is that if there is an error with that first CID, it will not be caught:
r := strings.NewReader("{} bad data")
var m map[string]interface{}
d := json.NewDecoder(r)
if err := d.Decode(&m); err != nil {
panic(err) // not triggered
}There was a problem hiding this comment.
the first entry would need to deserialize to a valid cid.
in the example the first entry {} deserializes to a valid map.
the fact remains: we're asking the server for a head. if they give us a failure / non-understandable response we fail availability. if they're purposefully corrupting their head response it's the same net result even if we go and ask for the CID here, so i'm not seeing this as causing any additional failure modes.
There was a problem hiding this comment.
Right, this is about a valid JSON value followed by garbage (or another JSON value) instead of EOF. Unmarshal catches that as an error, while Decode silently ignores the extra garbage.
This distinction is probably not hugely important in this particular scenario, so Will's response seems reasonable. We trust the response here. I think I'd still use Unmarshal for the sake of defensive programming, but I don't think it would actually make a difference in practice, unless the server has some serious bugs.
Conceptually, provide the same publisher and subscriber interface as legs, but with subscriber pulling from static files that the publisher makes available in an HTTP directory.
Syncto fetch.