Re: [PR] fix(puffin): reject trailing data in footer payloads [iceberg-go]

["laskoviymishka (via GitHub)" <gi...@apache.org>]

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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org