x/mod/sumdb: Server.ServeHTTP serves data tiles in different format than sum.golang.org #69348
Labels
help wanted
NeedsFix
The path to resolution is known, but the work has not been done.
proxy.golang.org
Milestone
Comments
timothy-king
added
the
NeedsInvestigation
|
CC @matloob @samthanawalla as owners of x/mod. |
|
cc @rsc, as author of this package. |
|
Thank you for the thorough report! Indeed, it looks like I doubt anyone is depending on the current format since no one noticed the discrepancy before. |
dmitshur
added
NeedsFix
|
Change https://go.dev/cl/618135 mentions this issue: |
gopherbot
pushed a commit
to golang/mod
that referenced
this issue
Nov 1, 2024
Make the format of sumdb.Server data tile responses compatible with those served by sum.golang.org: Like formatted records for the lookup endpoint, but without each record IDs. Updates documentation for sumdb/tlog.FormatRecord about data tiles. Server still calls FormatRecord to keep the validation, then removes the first line. For golang/go#69348 Change-Id: I1bea45b3343c58acc90982aaff5d41e32b06ae8c Reviewed-on: https://go-review.googlesource.com/c/mod/+/618135 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Matloob <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
|
@mjl- thanks for the CL. Should that have fixed this issue? |
|
@findleyr yes, thanks! closing the issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Go version
n/a (go1.23.1 on linux/amd64)
Output of
go envin your module/workspace:What did you do?
I used https://pkg.go.dev/golang.org/x/[email protected]/sumdb#Server.ServeHTTP to write a sumdb server for local testing.
I already had earlier code that parses data tiles (level -1 tiles) into records, and then verifies the hash of each record. It's part of gopherwatch, with the parsing at https://github.com/mjl-/gopherwatch/blob/3d50ff988dade2e571f3983c4194e451515a39d1/sums.go#L288. The parser is a bit sloppy, but it can verify the data tiles served by sum.golang.org.
Configuring gopherwatch to verify the data tiles of a sumdb server written with sumdb#Server.ServeHTTP was giving me unexpected verification errors (hashes of record data different than expected based on the tiles containing hashes).
It turns out that data tiles served by sum.golang.org have a different format than the data tiles served by golang.org/x/mod/sumdb.Server. In sumdb.Server, each record is prefixed with its record ID and a newline. On sum.golang.org they are not. The hashes of the records stored in the level 0 tiles are calculated over the record data without a record ID, that explains the errors I was seeing.
For example, a (random) data tile from sum.golang.org: https://sum.golang.org/tile/8/data/x115/673
It looks like this:
However, a data tile served by sumdb.Server would look like this (note the extra number before the record data: the record id):
The data tiles are composed at https://github.com/golang/mod/blob/46a3137daeac7bd5e64dc5971191e4a7207e6d89/sumdb/server.go#L148,
using tlog.FormatRecord from https://github.com/golang/mod/blob/46a3137daeac7bd5e64dc5971191e4a7207e6d89/sumdb/tlog/note.go#L75.
The comment on that function also says that format is used by a data tile. That is true for sumdb.Server, but not for sum.golang.org.
The record format (with signature) is used in the responses to individual module lookups, e.g. https://sum.golang.org/lookup/github.com/mjl-/[email protected].
The Go tooling may not be parsing the data tiles, working only on lookups and the tiles containing hashes. But I think it would be good to have sumdb.Server serve the same format as sum.golang.org for data tiles, so clients that can process data tiles from sum.golang.org can also parse data tiles served by a sumdb.Server.
There could be concerns about compatibility: existing code may now depend on the format currently served by sumdb.Server. But parsing data tiles doesn't seem very common.
If the format of sumdb.Server is deemed better and should stay, it would be good to document this difference. Clients parsing the data tiles could then recognize both formats.
What did you see happen?
sumdb.Server.ServeHTTP serving data tiles in a format different from what sum.golang.org is serving.
What did you expect to see?
sumdb.Server.ServeHTTP serving data tiles in the same format as sum.golang.org.
The text was updated successfully, but these errors were encountered: