fix(blob): verify the zlib trailer when the last byte is read (GO-001)
io.ReadFull and io.CopyN of exactly size bytes no longer miss a corrupt adler32 trailer: the last Read returns (0, ErrCorrupt) and withholds the final chunk. Close reports it when all bytes were consumed.
- commit
- cb638a629595eec2351cf5b2af384fa1b03bf0e0
- author
- brice · · 2026-10-11 01:07 UTC
- parent
- 3b41cc5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +193 -50
M blob.go +33 -8
| @@ -14,7 +14,12 @@ import ( | |||
| 14 | 14 | // rebuilt in a pooled buffer first and is bounded by WithMaxObjectSize | |
| 15 | 15 | // (ErrTooLarge). The caller must close the reader, which also releases the | |
| 16 | 16 | // pack it reads from. Reads detect truncated or oversized data and return | |
| 17 | - | // ErrCorrupt. | |
| 17 | + | // ErrCorrupt. The checksum of the stream is verified when the last byte is | |
| 18 | + | // read: that Read returns (0, ErrCorrupt) instead of the final chunk, so | |
| 19 | + | // io.ReadFull and io.CopyN of exactly size bytes report a corrupt trailer too. | |
| 20 | + | // Close returns ErrCorrupt if all size bytes were consumed (or the blob is | |
| 21 | + | // empty) but the end of the stream was not checked. A reader closed before | |
| 22 | + | // size bytes were read cannot verify the checksum. | |
| 18 | 23 | func (r *Repo) OpenBlob(id ObjectID) (int64, io.ReadCloser, error) { | |
| 19 | 24 | f, err := r.finder() | |
| 20 | 25 | if err != nil { | |
| @@ -155,25 +160,24 @@ func (s *streamBlob) Read(p []byte) (int, error) { | |||
| 155 | 160 | if int64(len(p)) > s.left { | |
| 156 | 161 | p = p[:s.left] | |
| 157 | 162 | } | |
| 158 | - | n := 0 | |
| 159 | 163 | if len(s.lead) > 0 { | |
| 160 | - | n = copy(p, s.lead) | |
| 164 | + | n := copy(p, s.lead) | |
| 161 | 165 | s.lead = s.lead[n:] | |
| 162 | 166 | s.left -= int64(n) | |
| 163 | - | return n, nil | |
| 167 | + | return s.served(n) | |
| 164 | 168 | } | |
| 165 | 169 | n, err := s.zr.Read(p) | |
| 166 | 170 | s.left -= int64(n) | |
| 167 | 171 | if err != nil && !errors.Is(err, io.EOF) { | |
| 168 | 172 | s.err = corrupt("blob: %v", err) | |
| 169 | - | return n, s.err | |
| 173 | + | return 0, s.err | |
| 170 | 174 | } | |
| 171 | 175 | if errors.Is(err, io.EOF) && s.left > 0 { | |
| 172 | 176 | s.err = corrupt("blob shorter than its declared size") | |
| 173 | - | return n, s.err | |
| 177 | + | return 0, s.err | |
| 174 | 178 | } | |
| 175 | 179 | if n > 0 { | |
| 176 | - | return n, nil | |
| 180 | + | return s.served(n) | |
| 177 | 181 | } | |
| 178 | 182 | if s.left == 0 { | |
| 179 | 183 | return 0, s.finish() | |
| @@ -181,6 +185,20 @@ func (s *streamBlob) Read(p []byte) (int, error) { | |||
| 181 | 185 | return 0, nil | |
| 182 | 186 | } | |
| 183 | 187 | ||
| 188 | + | // served returns the n > 0 bytes just read. When they are the last ones, it | |
| 189 | + | // verifies the end of the stream and its checksum before returning them, so a | |
| 190 | + | // caller that stops at the declared size (io.ReadFull, io.CopyN) still sees a | |
| 191 | + | // corrupt trailer. On failure the chunk is withheld, (0, err), because | |
| 192 | + | // io.ReadFull drops an error that comes with a full buffer. | |
| 193 | + | func (s *streamBlob) served(n int) (int, error) { | |
| 194 | + | if s.left == 0 { | |
| 195 | + | if err := s.finish(); !errors.Is(err, io.EOF) { | |
| 196 | + | return 0, err | |
| 197 | + | } | |
| 198 | + | } | |
| 199 | + | return n, nil | |
| 200 | + | } | |
| 201 | + | ||
| 184 | 202 | // finish checks that the stream ends exactly here and that its checksum is | |
| 185 | 203 | // right, then reports EOF. | |
| 186 | 204 | func (s *streamBlob) finish() error { | |
| @@ -204,6 +222,13 @@ func (s *streamBlob) Close() error { | |||
| 204 | 222 | if s.zr == nil { | |
| 205 | 223 | return nil | |
| 206 | 224 | } | |
| 225 | + | var err error | |
| 226 | + | if s.left == 0 && !s.done { | |
| 227 | + | // Nothing was read yet (an empty blob): verify the stream now. | |
| 228 | + | if err = s.finish(); errors.Is(err, io.EOF) { | |
| 229 | + | err = nil | |
| 230 | + | } | |
| 231 | + | } | |
| 207 | 232 | s.zr = nil | |
| 208 | 233 | if s.src != nil { | |
| 209 | 234 | putMem(s.src) | |
| @@ -212,5 +237,5 @@ func (s *streamBlob) Close() error { | |||
| 212 | 237 | if s.loose != nil { | |
| 213 | 238 | s.loose.close() | |
| 214 | 239 | } | |
| 215 | - | return nil | |
| 240 | + | return err | |
| 216 | 241 | } | |
| @@ -14,7 +14,12 @@ import ( | |||||
| 14 | // rebuilt in a pooled buffer first and is bounded by WithMaxObjectSize | 14 | // rebuilt in a pooled buffer first and is bounded by WithMaxObjectSize | ||
| 15 | // (ErrTooLarge). The caller must close the reader, which also releases the | 15 | // (ErrTooLarge). The caller must close the reader, which also releases the | ||
| 16 | // pack it reads from. Reads detect truncated or oversized data and return | 16 | // pack it reads from. Reads detect truncated or oversized data and return | ||
| 17 | - | // ErrCorrupt. | 17 | + | // ErrCorrupt. The checksum of the stream is verified when the last byte is |
| 18 | + | // read: that Read returns (0, ErrCorrupt) instead of the final chunk, so | |||
| 19 | + | // io.ReadFull and io.CopyN of exactly size bytes report a corrupt trailer too. | |||
| 20 | + | // Close returns ErrCorrupt if all size bytes were consumed (or the blob is | |||
| 21 | + | // empty) but the end of the stream was not checked. A reader closed before | |||
| 22 | + | // size bytes were read cannot verify the checksum. | |||
| 18 | func (r *Repo) OpenBlob(id ObjectID) (int64, io.ReadCloser, error) { | 23 | func (r *Repo) OpenBlob(id ObjectID) (int64, io.ReadCloser, error) { | ||
| 19 | f, err := r.finder() | 24 | f, err := r.finder() | ||
| 20 | if err != nil { | 25 | if err != nil { | ||
| @@ -155,25 +160,24 @@ func (s *streamBlob) Read(p []byte) (int, error) { | |||||
| 155 | if int64(len(p)) > s.left { | 160 | if int64(len(p)) > s.left { | ||
| 156 | p = p[:s.left] | 161 | p = p[:s.left] | ||
| 157 | } | 162 | } | ||
| 158 | - | n := 0 | |||
| 159 | if len(s.lead) > 0 { | 163 | if len(s.lead) > 0 { | ||
| 160 | - | n = copy(p, s.lead) | 164 | + | n := copy(p, s.lead) |
| 161 | s.lead = s.lead[n:] | 165 | s.lead = s.lead[n:] | ||
| 162 | s.left -= int64(n) | 166 | s.left -= int64(n) | ||
| 163 | - | return n, nil | 167 | + | return s.served(n) |
| 164 | } | 168 | } | ||
| 165 | n, err := s.zr.Read(p) | 169 | n, err := s.zr.Read(p) | ||
| 166 | s.left -= int64(n) | 170 | s.left -= int64(n) | ||
| 167 | if err != nil && !errors.Is(err, io.EOF) { | 171 | if err != nil && !errors.Is(err, io.EOF) { | ||
| 168 | s.err = corrupt("blob: %v", err) | 172 | s.err = corrupt("blob: %v", err) | ||
| 169 | - | return n, s.err | 173 | + | return 0, s.err |
| 170 | } | 174 | } | ||
| 171 | if errors.Is(err, io.EOF) && s.left > 0 { | 175 | if errors.Is(err, io.EOF) && s.left > 0 { | ||
| 172 | s.err = corrupt("blob shorter than its declared size") | 176 | s.err = corrupt("blob shorter than its declared size") | ||
| 173 | - | return n, s.err | 177 | + | return 0, s.err |
| 174 | } | 178 | } | ||
| 175 | if n > 0 { | 179 | if n > 0 { | ||
| 176 | - | return n, nil | 180 | + | return s.served(n) |
| 177 | } | 181 | } | ||
| 178 | if s.left == 0 { | 182 | if s.left == 0 { | ||
| 179 | return 0, s.finish() | 183 | return 0, s.finish() | ||
| @@ -181,6 +185,20 @@ func (s *streamBlob) Read(p []byte) (int, error) { | |||||
| 181 | return 0, nil | 185 | return 0, nil | ||
| 182 | } | 186 | } | ||
| 183 | 187 | ||||
| 188 | + | // served returns the n > 0 bytes just read. When they are the last ones, it | |||
| 189 | + | // verifies the end of the stream and its checksum before returning them, so a | |||
| 190 | + | // caller that stops at the declared size (io.ReadFull, io.CopyN) still sees a | |||
| 191 | + | // corrupt trailer. On failure the chunk is withheld, (0, err), because | |||
| 192 | + | // io.ReadFull drops an error that comes with a full buffer. | |||
| 193 | + | func (s *streamBlob) served(n int) (int, error) { | |||
| 194 | + | if s.left == 0 { | |||
| 195 | + | if err := s.finish(); !errors.Is(err, io.EOF) { | |||
| 196 | + | return 0, err | |||
| 197 | + | } | |||
| 198 | + | } | |||
| 199 | + | return n, nil | |||
| 200 | + | } | |||
| 201 | + | ||||
| 184 | // finish checks that the stream ends exactly here and that its checksum is | 202 | // finish checks that the stream ends exactly here and that its checksum is | ||
| 185 | // right, then reports EOF. | 203 | // right, then reports EOF. | ||
| 186 | func (s *streamBlob) finish() error { | 204 | func (s *streamBlob) finish() error { | ||
| @@ -204,6 +222,13 @@ func (s *streamBlob) Close() error { | |||||
| 204 | if s.zr == nil { | 222 | if s.zr == nil { | ||
| 205 | return nil | 223 | return nil | ||
| 206 | } | 224 | } | ||
| 225 | + | var err error | |||
| 226 | + | if s.left == 0 && !s.done { | |||
| 227 | + | // Nothing was read yet (an empty blob): verify the stream now. | |||
| 228 | + | if err = s.finish(); errors.Is(err, io.EOF) { | |||
| 229 | + | err = nil | |||
| 230 | + | } | |||
| 231 | + | } | |||
| 207 | s.zr = nil | 232 | s.zr = nil | ||
| 208 | if s.src != nil { | 233 | if s.src != nil { | ||
| 209 | putMem(s.src) | 234 | putMem(s.src) | ||
| @@ -212,5 +237,5 @@ func (s *streamBlob) Close() error { | |||||
| 212 | if s.loose != nil { | 237 | if s.loose != nil { | ||
| 213 | s.loose.close() | 238 | s.loose.close() | ||
| 214 | } | 239 | } | ||
| 215 | - | return nil | 240 | + | return err |
| 216 | } | 241 | } | ||
M inflate_test.go +160 -42
| @@ -213,6 +213,54 @@ func flipByteAt(t *testing.T, path string, off int64) { | |||
| 213 | 213 | } | |
| 214 | 214 | } | |
| 215 | 215 | ||
| 216 | + | // corruptObject flips a bit in the deflate data or in the adler32 trailer of | |
| 217 | + | // the loose or packed object id (packed: repack first). | |
| 218 | + | func corruptObject(t *testing.T, f *fx, id ObjectID, packed bool, where string) { | |
| 219 | + | t.Helper() | |
| 220 | + | var file string | |
| 221 | + | var entryEnd int64 = -1 | |
| 222 | + | if packed { | |
| 223 | + | f.git("repack", "-adf") | |
| 224 | + | f.git("prune-packed") | |
| 225 | + | matches, _ := filepath.Glob(filepath.Join(f.dir, "objects/pack/*.pack")) | |
| 226 | + | if len(matches) != 1 { | |
| 227 | + | t.Fatalf("packs: %v", matches) | |
| 228 | + | } | |
| 229 | + | file = matches[0] | |
| 230 | + | // The blob's entry ends where the next one starts, or at | |
| 231 | + | // the pack trailer. | |
| 232 | + | r := f.open() | |
| 233 | + | p := r.snap.Load().packs[0] | |
| 234 | + | pos, ok := p.idx.find(&id) | |
| 235 | + | if !ok { | |
| 236 | + | t.Fatal("blob not in the pack") | |
| 237 | + | } | |
| 238 | + | off, _ := p.idx.offset(pos) | |
| 239 | + | next := uint64(len(p.data) - packTrailerLen) | |
| 240 | + | for i := range p.idx.n { | |
| 241 | + | if o, _ := p.idx.offset(i); o > off && o < next { | |
| 242 | + | next = o | |
| 243 | + | } | |
| 244 | + | } | |
| 245 | + | entryEnd = int64(next) | |
| 246 | + | _ = r.Close() | |
| 247 | + | } else { | |
| 248 | + | f.git("prune-packed") | |
| 249 | + | file = filepath.Join(f.dir, "objects", id.String()[:2], id.String()[2:]) | |
| 250 | + | } | |
| 251 | + | switch { | |
| 252 | + | case where == "trailer" && packed: | |
| 253 | + | flipByteAt(t, file, entryEnd-1) | |
| 254 | + | case where == "trailer": | |
| 255 | + | flipByteAt(t, file, -1) | |
| 256 | + | case packed: | |
| 257 | + | flipByteAt(t, file, entryEnd/2+6) // inside the deflate data | |
| 258 | + | default: | |
| 259 | + | st, _ := os.Stat(file) | |
| 260 | + | flipByteAt(t, file, st.Size()/2) | |
| 261 | + | } | |
| 262 | + | } | |
| 263 | + | ||
| 216 | 264 | // A flipped byte in the compressed content or in the adler32 trailer of a | |
| 217 | 265 | // loose or a packed object is ErrCorrupt on every way to read it, since the | |
| 218 | 266 | // checksum is the only check of the inflated content. | |
| @@ -225,48 +273,7 @@ func TestCorruptedObjectsAreErrCorrupt(t *testing.T) { | |||
| 225 | 273 | f.write("f.txt", content, 0o644) | |
| 226 | 274 | f.commit("one") | |
| 227 | 275 | id := mustID(t, f.git("rev-parse", "HEAD:f.txt")) | |
| 228 | - | var file string | |
| 229 | - | var entryEnd int64 = -1 | |
| 230 | - | if packed { | |
| 231 | - | f.git("repack", "-adf") | |
| 232 | - | f.git("prune-packed") | |
| 233 | - | matches, _ := filepath.Glob(filepath.Join(f.dir, "objects/pack/*.pack")) | |
| 234 | - | if len(matches) != 1 { | |
| 235 | - | t.Fatalf("packs: %v", matches) | |
| 236 | - | } | |
| 237 | - | file = matches[0] | |
| 238 | - | // The blob's entry ends where the next one starts, or at | |
| 239 | - | // the pack trailer. | |
| 240 | - | r := f.open() | |
| 241 | - | p := r.snap.Load().packs[0] | |
| 242 | - | pos, ok := p.idx.find(&id) | |
| 243 | - | if !ok { | |
| 244 | - | t.Fatal("blob not in the pack") | |
| 245 | - | } | |
| 246 | - | off, _ := p.idx.offset(pos) | |
| 247 | - | next := uint64(len(p.data) - packTrailerLen) | |
| 248 | - | for i := range p.idx.n { | |
| 249 | - | if o, _ := p.idx.offset(i); o > off && o < next { | |
| 250 | - | next = o | |
| 251 | - | } | |
| 252 | - | } | |
| 253 | - | entryEnd = int64(next) | |
| 254 | - | _ = r.Close() | |
| 255 | - | } else { | |
| 256 | - | f.git("prune-packed") | |
| 257 | - | file = filepath.Join(f.dir, "objects", id.String()[:2], id.String()[2:]) | |
| 258 | - | } | |
| 259 | - | switch { | |
| 260 | - | case where == "trailer" && packed: | |
| 261 | - | flipByteAt(t, file, entryEnd-1) | |
| 262 | - | case where == "trailer": | |
| 263 | - | flipByteAt(t, file, -1) | |
| 264 | - | case packed: | |
| 265 | - | flipByteAt(t, file, entryEnd/2+6) // inside the deflate data | |
| 266 | - | default: | |
| 267 | - | st, _ := os.Stat(file) | |
| 268 | - | flipByteAt(t, file, st.Size()/2) | |
| 269 | - | } | |
| 276 | + | corruptObject(t, f, id, packed, where) | |
| 270 | 277 | r := f.open() | |
| 271 | 278 | if _, _, err := r.ReadObject(id, nil); !errors.Is(err, ErrCorrupt) { | |
| 272 | 279 | t.Errorf("ReadObject = %v, want ErrCorrupt", err) | |
| @@ -283,3 +290,114 @@ func TestCorruptedObjectsAreErrCorrupt(t *testing.T) { | |||
| 283 | 290 | } | |
| 284 | 291 | } | |
| 285 | 292 | } | |
| 293 | + | ||
| 294 | + | // A corrupt adler32 trailer is reported by readers that stop at the declared | |
| 295 | + | // size too (io.ReadFull, io.CopyN), not only by those that read to EOF, and | |
| 296 | + | // the last chunk is not returned along with a nil error. | |
| 297 | + | func TestOpenBlobTrailerCorruptionOnExactReads(t *testing.T) { | |
| 298 | + | for _, packed := range []bool{false, true} { | |
| 299 | + | for _, size := range []int{0, 1, 30, 20000} { | |
| 300 | + | t.Run(fmt.Sprintf("packed=%v/size=%d", packed, size), func(t *testing.T) { | |
| 301 | + | f := newFx(t) | |
| 302 | + | content := textLines(size/16+1, 4)[:size] | |
| 303 | + | f.write("f.txt", content, 0o644) | |
| 304 | + | f.commit("one") | |
| 305 | + | id := mustID(t, f.git("rev-parse", "HEAD:f.txt")) | |
| 306 | + | corruptObject(t, f, id, packed, "trailer") | |
| 307 | + | r := f.open() | |
| 308 | + | open := func() io.ReadCloser { | |
| 309 | + | n, rc, err := r.OpenBlob(id) | |
| 310 | + | if err != nil || n != int64(size) { | |
| 311 | + | t.Fatalf("OpenBlob = %d %v", n, err) | |
| 312 | + | } | |
| 313 | + | return rc | |
| 314 | + | } | |
| 315 | + | if size > 0 { | |
| 316 | + | rc := open() | |
| 317 | + | buf := make([]byte, size) | |
| 318 | + | if n, err := io.ReadFull(rc, buf); !errors.Is(err, ErrCorrupt) { | |
| 319 | + | t.Errorf("io.ReadFull = %d %v, want ErrCorrupt", n, err) | |
| 320 | + | } | |
| 321 | + | _ = rc.Close() | |
| 322 | + | ||
| 323 | + | rc = open() | |
| 324 | + | if n, err := io.CopyN(io.Discard, rc, int64(size)); !errors.Is(err, ErrCorrupt) { | |
| 325 | + | t.Errorf("io.CopyN = %d %v, want ErrCorrupt", n, err) | |
| 326 | + | } | |
| 327 | + | _ = rc.Close() | |
| 328 | + | ||
| 329 | + | rc = open() | |
| 330 | + | if _, err := io.Copy(io.Discard, rc); !errors.Is(err, ErrCorrupt) { | |
| 331 | + | t.Errorf("io.Copy = %v, want ErrCorrupt", err) | |
| 332 | + | } | |
| 333 | + | // The error is sticky. | |
| 334 | + | if _, err := rc.Read(make([]byte, 1)); !errors.Is(err, ErrCorrupt) { | |
| 335 | + | t.Errorf("Read after the error = %v, want ErrCorrupt", err) | |
| 336 | + | } | |
| 337 | + | _ = rc.Close() | |
| 338 | + | ||
| 339 | + | // One byte at a time: the last Read fails. | |
| 340 | + | rc = open() | |
| 341 | + | var err error | |
| 342 | + | one := make([]byte, 1) | |
| 343 | + | for i := 0; i < size && err == nil; i++ { | |
| 344 | + | _, err = rc.Read(one) | |
| 345 | + | } | |
| 346 | + | if !errors.Is(err, ErrCorrupt) { | |
| 347 | + | t.Errorf("byte-wise read = %v, want ErrCorrupt", err) | |
| 348 | + | } | |
| 349 | + | _ = rc.Close() | |
| 350 | + | } | |
| 351 | + | if size == 0 { | |
| 352 | + | // Nothing is read from an empty blob: Close reports it. | |
| 353 | + | rc := open() | |
| 354 | + | if err := rc.Close(); !errors.Is(err, ErrCorrupt) { | |
| 355 | + | t.Errorf("Close = %v, want ErrCorrupt", err) | |
| 356 | + | } | |
| 357 | + | } | |
| 358 | + | }) | |
| 359 | + | } | |
| 360 | + | } | |
| 361 | + | } | |
| 362 | + | ||
| 363 | + | // An intact blob read to exactly its size is returned whole and Close is nil, | |
| 364 | + | // and an early Close is not an error. | |
| 365 | + | func TestOpenBlobExactReadsIntact(t *testing.T) { | |
| 366 | + | for _, packed := range []bool{false, true} { | |
| 367 | + | f := newFx(t) | |
| 368 | + | content := textLines(1500, 5) | |
| 369 | + | f.write("f.txt", content, 0o644) | |
| 370 | + | f.write("empty", nil, 0o644) | |
| 371 | + | f.commit("one") | |
| 372 | + | if packed { | |
| 373 | + | f.git("repack", "-adf") | |
| 374 | + | } | |
| 375 | + | r := f.open() | |
| 376 | + | id := mustID(t, f.git("rev-parse", "HEAD:f.txt")) | |
| 377 | + | n, rc, err := r.OpenBlob(id) | |
| 378 | + | if err != nil { | |
| 379 | + | t.Fatal(err) | |
| 380 | + | } | |
| 381 | + | buf := make([]byte, n) | |
| 382 | + | if _, err := io.ReadFull(rc, buf); err != nil || !bytes.Equal(buf, content) { | |
| 383 | + | t.Fatalf("packed=%v: ReadFull: %v", packed, err) | |
| 384 | + | } | |
| 385 | + | if err := rc.Close(); err != nil { | |
| 386 | + | t.Errorf("Close = %v", err) | |
| 387 | + | } | |
| 388 | + | _, rc, _ = r.OpenBlob(id) | |
| 389 | + | if _, err := io.CopyN(io.Discard, rc, 10); err != nil { | |
| 390 | + | t.Fatal(err) | |
| 391 | + | } | |
| 392 | + | if err := rc.Close(); err != nil { | |
| 393 | + | t.Errorf("early Close = %v", err) | |
| 394 | + | } | |
| 395 | + | _, rc, err = r.OpenBlob(mustID(t, f.git("rev-parse", "HEAD:empty"))) | |
| 396 | + | if err != nil { | |
| 397 | + | t.Fatal(err) | |
| 398 | + | } | |
| 399 | + | if err := rc.Close(); err != nil { | |
| 400 | + | t.Errorf("Close of an intact empty blob = %v", err) | |
| 401 | + | } | |
| 402 | + | } | |
| 403 | + | } | |
| @@ -213,6 +213,54 @@ func flipByteAt(t *testing.T, path string, off int64) { | |||||
| 213 | } | 213 | } | ||
| 214 | } | 214 | } | ||
| 215 | 215 | ||||
| 216 | + | // corruptObject flips a bit in the deflate data or in the adler32 trailer of | |||
| 217 | + | // the loose or packed object id (packed: repack first). | |||
| 218 | + | func corruptObject(t *testing.T, f *fx, id ObjectID, packed bool, where string) { | |||
| 219 | + | t.Helper() | |||
| 220 | + | var file string | |||
| 221 | + | var entryEnd int64 = -1 | |||
| 222 | + | if packed { | |||
| 223 | + | f.git("repack", "-adf") | |||
| 224 | + | f.git("prune-packed") | |||
| 225 | + | matches, _ := filepath.Glob(filepath.Join(f.dir, "objects/pack/*.pack")) | |||
| 226 | + | if len(matches) != 1 { | |||
| 227 | + | t.Fatalf("packs: %v", matches) | |||
| 228 | + | } | |||
| 229 | + | file = matches[0] | |||
| 230 | + | // The blob's entry ends where the next one starts, or at | |||
| 231 | + | // the pack trailer. | |||
| 232 | + | r := f.open() | |||
| 233 | + | p := r.snap.Load().packs[0] | |||
| 234 | + | pos, ok := p.idx.find(&id) | |||
| 235 | + | if !ok { | |||
| 236 | + | t.Fatal("blob not in the pack") | |||
| 237 | + | } | |||
| 238 | + | off, _ := p.idx.offset(pos) | |||
| 239 | + | next := uint64(len(p.data) - packTrailerLen) | |||
| 240 | + | for i := range p.idx.n { | |||
| 241 | + | if o, _ := p.idx.offset(i); o > off && o < next { | |||
| 242 | + | next = o | |||
| 243 | + | } | |||
| 244 | + | } | |||
| 245 | + | entryEnd = int64(next) | |||
| 246 | + | _ = r.Close() | |||
| 247 | + | } else { | |||
| 248 | + | f.git("prune-packed") | |||
| 249 | + | file = filepath.Join(f.dir, "objects", id.String()[:2], id.String()[2:]) | |||
| 250 | + | } | |||
| 251 | + | switch { | |||
| 252 | + | case where == "trailer" && packed: | |||
| 253 | + | flipByteAt(t, file, entryEnd-1) | |||
| 254 | + | case where == "trailer": | |||
| 255 | + | flipByteAt(t, file, -1) | |||
| 256 | + | case packed: | |||
| 257 | + | flipByteAt(t, file, entryEnd/2+6) // inside the deflate data | |||
| 258 | + | default: | |||
| 259 | + | st, _ := os.Stat(file) | |||
| 260 | + | flipByteAt(t, file, st.Size()/2) | |||
| 261 | + | } | |||
| 262 | + | } | |||
| 263 | + | ||||
| 216 | // A flipped byte in the compressed content or in the adler32 trailer of a | 264 | // A flipped byte in the compressed content or in the adler32 trailer of a | ||
| 217 | // loose or a packed object is ErrCorrupt on every way to read it, since the | 265 | // loose or a packed object is ErrCorrupt on every way to read it, since the | ||
| 218 | // checksum is the only check of the inflated content. | 266 | // checksum is the only check of the inflated content. | ||
| @@ -225,48 +273,7 @@ func TestCorruptedObjectsAreErrCorrupt(t *testing.T) { | |||||
| 225 | f.write("f.txt", content, 0o644) | 273 | f.write("f.txt", content, 0o644) | ||
| 226 | f.commit("one") | 274 | f.commit("one") | ||
| 227 | id := mustID(t, f.git("rev-parse", "HEAD:f.txt")) | 275 | id := mustID(t, f.git("rev-parse", "HEAD:f.txt")) | ||
| 228 | - | var file string | 276 | + | corruptObject(t, f, id, packed, where) |
| 229 | - | var entryEnd int64 = -1 | |||
| 230 | - | if packed { | |||
| 231 | - | f.git("repack", "-adf") | |||
| 232 | - | f.git("prune-packed") | |||
| 233 | - | matches, _ := filepath.Glob(filepath.Join(f.dir, "objects/pack/*.pack")) | |||
| 234 | - | if len(matches) != 1 { | |||
| 235 | - | t.Fatalf("packs: %v", matches) | |||
| 236 | - | } | |||
| 237 | - | file = matches[0] | |||
| 238 | - | // The blob's entry ends where the next one starts, or at | |||
| 239 | - | // the pack trailer. | |||
| 240 | - | r := f.open() | |||
| 241 | - | p := r.snap.Load().packs[0] | |||
| 242 | - | pos, ok := p.idx.find(&id) | |||
| 243 | - | if !ok { | |||
| 244 | - | t.Fatal("blob not in the pack") | |||
| 245 | - | } | |||
| 246 | - | off, _ := p.idx.offset(pos) | |||
| 247 | - | next := uint64(len(p.data) - packTrailerLen) | |||
| 248 | - | for i := range p.idx.n { | |||
| 249 | - | if o, _ := p.idx.offset(i); o > off && o < next { | |||
| 250 | - | next = o | |||
| 251 | - | } | |||
| 252 | - | } | |||
| 253 | - | entryEnd = int64(next) | |||
| 254 | - | _ = r.Close() | |||
| 255 | - | } else { | |||
| 256 | - | f.git("prune-packed") | |||
| 257 | - | file = filepath.Join(f.dir, "objects", id.String()[:2], id.String()[2:]) | |||
| 258 | - | } | |||
| 259 | - | switch { | |||
| 260 | - | case where == "trailer" && packed: | |||
| 261 | - | flipByteAt(t, file, entryEnd-1) | |||
| 262 | - | case where == "trailer": | |||
| 263 | - | flipByteAt(t, file, -1) | |||
| 264 | - | case packed: | |||
| 265 | - | flipByteAt(t, file, entryEnd/2+6) // inside the deflate data | |||
| 266 | - | default: | |||
| 267 | - | st, _ := os.Stat(file) | |||
| 268 | - | flipByteAt(t, file, st.Size()/2) | |||
| 269 | - | } | |||
| 270 | r := f.open() | 277 | r := f.open() | ||
| 271 | if _, _, err := r.ReadObject(id, nil); !errors.Is(err, ErrCorrupt) { | 278 | if _, _, err := r.ReadObject(id, nil); !errors.Is(err, ErrCorrupt) { | ||
| 272 | t.Errorf("ReadObject = %v, want ErrCorrupt", err) | 279 | t.Errorf("ReadObject = %v, want ErrCorrupt", err) | ||
| @@ -283,3 +290,114 @@ func TestCorruptedObjectsAreErrCorrupt(t *testing.T) { | |||||
| 283 | } | 290 | } | ||
| 284 | } | 291 | } | ||
| 285 | } | 292 | } | ||
| 293 | + | ||||
| 294 | + | // A corrupt adler32 trailer is reported by readers that stop at the declared | |||
| 295 | + | // size too (io.ReadFull, io.CopyN), not only by those that read to EOF, and | |||
| 296 | + | // the last chunk is not returned along with a nil error. | |||
| 297 | + | func TestOpenBlobTrailerCorruptionOnExactReads(t *testing.T) { | |||
| 298 | + | for _, packed := range []bool{false, true} { | |||
| 299 | + | for _, size := range []int{0, 1, 30, 20000} { | |||
| 300 | + | t.Run(fmt.Sprintf("packed=%v/size=%d", packed, size), func(t *testing.T) { | |||
| 301 | + | f := newFx(t) | |||
| 302 | + | content := textLines(size/16+1, 4)[:size] | |||
| 303 | + | f.write("f.txt", content, 0o644) | |||
| 304 | + | f.commit("one") | |||
| 305 | + | id := mustID(t, f.git("rev-parse", "HEAD:f.txt")) | |||
| 306 | + | corruptObject(t, f, id, packed, "trailer") | |||
| 307 | + | r := f.open() | |||
| 308 | + | open := func() io.ReadCloser { | |||
| 309 | + | n, rc, err := r.OpenBlob(id) | |||
| 310 | + | if err != nil || n != int64(size) { | |||
| 311 | + | t.Fatalf("OpenBlob = %d %v", n, err) | |||
| 312 | + | } | |||
| 313 | + | return rc | |||
| 314 | + | } | |||
| 315 | + | if size > 0 { | |||
| 316 | + | rc := open() | |||
| 317 | + | buf := make([]byte, size) | |||
| 318 | + | if n, err := io.ReadFull(rc, buf); !errors.Is(err, ErrCorrupt) { | |||
| 319 | + | t.Errorf("io.ReadFull = %d %v, want ErrCorrupt", n, err) | |||
| 320 | + | } | |||
| 321 | + | _ = rc.Close() | |||
| 322 | + | ||||
| 323 | + | rc = open() | |||
| 324 | + | if n, err := io.CopyN(io.Discard, rc, int64(size)); !errors.Is(err, ErrCorrupt) { | |||
| 325 | + | t.Errorf("io.CopyN = %d %v, want ErrCorrupt", n, err) | |||
| 326 | + | } | |||
| 327 | + | _ = rc.Close() | |||
| 328 | + | ||||
| 329 | + | rc = open() | |||
| 330 | + | if _, err := io.Copy(io.Discard, rc); !errors.Is(err, ErrCorrupt) { | |||
| 331 | + | t.Errorf("io.Copy = %v, want ErrCorrupt", err) | |||
| 332 | + | } | |||
| 333 | + | // The error is sticky. | |||
| 334 | + | if _, err := rc.Read(make([]byte, 1)); !errors.Is(err, ErrCorrupt) { | |||
| 335 | + | t.Errorf("Read after the error = %v, want ErrCorrupt", err) | |||
| 336 | + | } | |||
| 337 | + | _ = rc.Close() | |||
| 338 | + | ||||
| 339 | + | // One byte at a time: the last Read fails. | |||
| 340 | + | rc = open() | |||
| 341 | + | var err error | |||
| 342 | + | one := make([]byte, 1) | |||
| 343 | + | for i := 0; i < size && err == nil; i++ { | |||
| 344 | + | _, err = rc.Read(one) | |||
| 345 | + | } | |||
| 346 | + | if !errors.Is(err, ErrCorrupt) { | |||
| 347 | + | t.Errorf("byte-wise read = %v, want ErrCorrupt", err) | |||
| 348 | + | } | |||
| 349 | + | _ = rc.Close() | |||
| 350 | + | } | |||
| 351 | + | if size == 0 { | |||
| 352 | + | // Nothing is read from an empty blob: Close reports it. | |||
| 353 | + | rc := open() | |||
| 354 | + | if err := rc.Close(); !errors.Is(err, ErrCorrupt) { | |||
| 355 | + | t.Errorf("Close = %v, want ErrCorrupt", err) | |||
| 356 | + | } | |||
| 357 | + | } | |||
| 358 | + | }) | |||
| 359 | + | } | |||
| 360 | + | } | |||
| 361 | + | } | |||
| 362 | + | ||||
| 363 | + | // An intact blob read to exactly its size is returned whole and Close is nil, | |||
| 364 | + | // and an early Close is not an error. | |||
| 365 | + | func TestOpenBlobExactReadsIntact(t *testing.T) { | |||
| 366 | + | for _, packed := range []bool{false, true} { | |||
| 367 | + | f := newFx(t) | |||
| 368 | + | content := textLines(1500, 5) | |||
| 369 | + | f.write("f.txt", content, 0o644) | |||
| 370 | + | f.write("empty", nil, 0o644) | |||
| 371 | + | f.commit("one") | |||
| 372 | + | if packed { | |||
| 373 | + | f.git("repack", "-adf") | |||
| 374 | + | } | |||
| 375 | + | r := f.open() | |||
| 376 | + | id := mustID(t, f.git("rev-parse", "HEAD:f.txt")) | |||
| 377 | + | n, rc, err := r.OpenBlob(id) | |||
| 378 | + | if err != nil { | |||
| 379 | + | t.Fatal(err) | |||
| 380 | + | } | |||
| 381 | + | buf := make([]byte, n) | |||
| 382 | + | if _, err := io.ReadFull(rc, buf); err != nil || !bytes.Equal(buf, content) { | |||
| 383 | + | t.Fatalf("packed=%v: ReadFull: %v", packed, err) | |||
| 384 | + | } | |||
| 385 | + | if err := rc.Close(); err != nil { | |||
| 386 | + | t.Errorf("Close = %v", err) | |||
| 387 | + | } | |||
| 388 | + | _, rc, _ = r.OpenBlob(id) | |||
| 389 | + | if _, err := io.CopyN(io.Discard, rc, 10); err != nil { | |||
| 390 | + | t.Fatal(err) | |||
| 391 | + | } | |||
| 392 | + | if err := rc.Close(); err != nil { | |||
| 393 | + | t.Errorf("early Close = %v", err) | |||
| 394 | + | } | |||
| 395 | + | _, rc, err = r.OpenBlob(mustID(t, f.git("rev-parse", "HEAD:empty"))) | |||
| 396 | + | if err != nil { | |||
| 397 | + | t.Fatal(err) | |||
| 398 | + | } | |||
| 399 | + | if err := rc.Close(); err != nil { | |||
| 400 | + | t.Errorf("Close of an intact empty blob = %v", err) | |||
| 401 | + | } | |||
| 402 | + | } | |||
| 403 | + | } | |||