laskoviymishka commented on code in PR #1580:
URL: https://github.com/apache/iceberg-go/pull/1580#discussion_r3719805346
##########
puffin/puffin_reader.go:
##########
@@ -224,11 +224,18 @@ func (r *Reader) readFooter() error {
}
payloadReader := io.NewSectionReader(r.r, footerStart+MagicSize,
payloadSize)
+ decoder := json.NewDecoder(payloadReader)
var footer Footer
- if err := json.NewDecoder(payloadReader).Decode(&footer); err != nil {
+ if err := decoder.Decode(&footer); err != nil {
return fmt.Errorf("puffin: decode footer JSON: %w", err)
}
+ if _, err := decoder.Token(); err == nil {
Review Comment:
Could we use `decoder.More()` here instead of `Token()`? It collapses this
into a single check — `More()` is true when there's any non-whitespace left in
the payload and false at a clean EOF, so we drop the three-way `err == nil` /
`io.EOF` / else branch entirely.
It'd also make the messages honest. Right now `{"blobs":[]}42` (a bare
scalar) and a truncated `{"blobs":[]} {` both report "multiple JSON values",
even though neither is a complete second value — with `More()` it's one path
and one message like `unexpected content after footer JSON`. wdyt?
##########
puffin/puffin_test.go:
##########
@@ -580,6 +590,33 @@ func TestReaderInvalidFile(t *testing.T) {
})
}
+func TestReaderRejectsTrailingFooterData(t *testing.T) {
+ t.Parallel()
Review Comment:
Small one — only the outer test calls `t.Parallel()`, so the table rows
still run serially inside it. And the sibling `TestReaderInvalidFile` doesn't
call it at all, so we've got two patterns in the same file now. I'd either add
`t.Parallel()` inside each `t.Run` or drop it from the outer test to match —
either's fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
