From d314d1f7e933ca83081eec289aa0cb6e75a7eeac Mon Sep 17 00:00:00 2001 From: Runxi Yu Date: Sun, 22 Mar 2026 17:33:27 +0000 Subject: objectstore{,/packed}: Document lifetime and integrity behavior --- objectstore/objectstore.go | 5 +++++ objectstore/packed/close.go | 10 ++-------- objectstore/packed/read_header.go | 4 ++++ objectstore/packed/read_reader.go | 19 ++++++++++++++++++- objectstore/packed/read_size.go | 4 ++++ objectstore/packed/read_test.go | 5 ----- objectstore/packed/store.go | 2 -- 7 files changed, 33 insertions(+), 16 deletions(-) (limited to 'objectstore') diff --git a/objectstore/objectstore.go b/objectstore/objectstore.go index f152a7f0..c3de12a5 100644 --- a/objectstore/objectstore.go +++ b/objectstore/objectstore.go @@ -19,6 +19,9 @@ import ( var ErrObjectNotFound = errors.New("objectstore: object not found") // Store reads Git objects by object ID. +// +// Unless an implementation explicitly documents otherwise, values returned by +// Store methods are only valid until the store is closed. type Store interface { // ReadBytesFull reads a full serialized object as "type size\0content". // @@ -39,6 +42,7 @@ type Store interface { // ReadReaderFull reads a full serialized object stream as "type size\0content". // // Caller must close the returned reader. + // The returned reader is only valid until the store is closed. // // Any read-time integrity verification performed while producing the stream // is implementation-defined. @@ -48,6 +52,7 @@ type Store interface { // and content stream. // // Caller must close the returned reader. + // The returned reader is only valid until the store is closed. // // Any read-time integrity verification performed while producing the stream // is implementation-defined. diff --git a/objectstore/packed/close.go b/objectstore/packed/close.go index 2bb98232..d83245c5 100644 --- a/objectstore/packed/close.go +++ b/objectstore/packed/close.go @@ -1,16 +1,10 @@ package packed // Close releases mapped pack/index resources associated with the store. +// +// Repeated calls to Close are undefined behavior. func (store *Store) Close() error { store.stateMu.Lock() - - if store.closed { - store.stateMu.Unlock() - - return nil - } - - store.closed = true root := store.root packs := store.packs store.stateMu.Unlock() diff --git a/objectstore/packed/read_header.go b/objectstore/packed/read_header.go index 5eb37c92..5070c98b 100644 --- a/objectstore/packed/read_header.go +++ b/objectstore/packed/read_header.go @@ -6,6 +6,10 @@ import ( ) // ReadHeader reads an object's type and declared content size. +// +// It resolves header metadata only. It does not verify that the full pack entry +// payload is readable and does not verify any zlib Adler-32 trailer for +// compressed entry data. func (store *Store) ReadHeader(id objectid.ObjectID) (objecttype.Type, int64, error) { loc, err := store.lookup(id) if err != nil { diff --git a/objectstore/packed/read_reader.go b/objectstore/packed/read_reader.go index cbecaf2c..5c9ac8f4 100644 --- a/objectstore/packed/read_reader.go +++ b/objectstore/packed/read_reader.go @@ -12,9 +12,18 @@ import ( packfmt "codeberg.org/lindenii/furgit/packfile" ) -// ReadReaderContent reads an object's type, declared content size, and content stream. +// ReadReaderContent reads an object's type, declared content size, and content +// stream. // // The caller must close the returned reader. +// +// For base pack entries, the returned reader borrows store-owned mapped pack +// data and is only valid until the store is closed. +// +// Close releases reader-local resources only. It does not drain unread data for +// additional validation. In particular, malformed trailing compressed data, +// trailing bytes past the declared object size, and the zlib Adler-32 trailer +// may go unverified unless the caller reads to io.EOF. func (store *Store) ReadReaderContent(id objectid.ObjectID) (objecttype.Type, int64, io.ReadCloser, error) { loc, err := store.lookup(id) if err != nil { @@ -49,6 +58,14 @@ func (store *Store) ReadReaderContent(id objectid.ObjectID) (objecttype.Type, in // ReadReaderFull reads a full serialized object stream as "type size\0content". // // The caller must close the returned reader. +// +// For base pack entries, the returned reader borrows store-owned mapped pack +// data and is only valid until the store is closed. +// +// Close releases reader-local resources only. It does not drain unread data for +// additional validation. In particular, malformed trailing compressed data, +// trailing bytes past the declared object size, and the zlib Adler-32 trailer +// may go unverified unless the caller reads to io.EOF. func (store *Store) ReadReaderFull(id objectid.ObjectID) (io.ReadCloser, error) { loc, err := store.lookup(id) if err != nil { diff --git a/objectstore/packed/read_size.go b/objectstore/packed/read_size.go index 8581b01b..4afa6fed 100644 --- a/objectstore/packed/read_size.go +++ b/objectstore/packed/read_size.go @@ -9,6 +9,10 @@ import ( ) // ReadSize reads an object's declared content size. +// +// Like ReadHeader, it resolves header metadata only. It does not verify that +// the full pack entry payload is readable and does not verify any zlib +// Adler-32 trailer for compressed entry data. func (store *Store) ReadSize(id objectid.ObjectID) (int64, error) { loc, err := store.lookup(id) if err != nil { diff --git a/objectstore/packed/read_test.go b/objectstore/packed/read_test.go index 0e5da5d8..2f6b4979 100644 --- a/objectstore/packed/read_test.go +++ b/objectstore/packed/read_test.go @@ -177,11 +177,6 @@ func TestPackedStoreNewValidation(t *testing.T) { if err != nil { t.Fatalf("Close: %v", err) } - - err = store.Close() - if err != nil { - t.Fatalf("Close second: %v", err) - } }) } diff --git a/objectstore/packed/store.go b/objectstore/packed/store.go index 1c6082f6..58534709 100644 --- a/objectstore/packed/store.go +++ b/objectstore/packed/store.go @@ -46,8 +46,6 @@ type Store struct { packs map[string]*packFile // deltaCache caches resolved base objects by pack location. deltaCache *deltaCache - // closed reports whether Close has been called. - closed bool } var _ objectstore.Store = (*Store)(nil) -- cgit v1.3.1-10-gc9f91