Refuse objects the store could not read back a9f686edf60b
Found by the Linux benchmark: a first commit of the Linux tree made a 1.8 GB change, which the store packed although it exceeds the 1 GiB an object may be; every later command then refused to open the repository with 'index entry points outside the pack'. put now refuses an object over the limit before anything is written, with a message saying what to do, and the index loader names an oversized entry for what it is. The design limit (every inserted line inline in its change) is #34. closes #33 refs #34
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-09-29 19:22 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
a9f686edf60b2277cbd68abe4e83a3eb60b77992ee07145cc20a23a5a49503f1- Depends on
- 2b872d074ddb 75167672ebcd 8f10a07a1dbe afaba5f1306d ff0af1ee299d
- Depended on by
- 767ac6cd0545 092457020692 54e2e6309aa3 648189388f1a 6f8aeb996558
- In bookmarks
- agents-in-readme bare-hashes carried-not-removed carry-identical doctor-trust identical-sides keyless-tags main merge-remote-bookmark only-help-103 paths-for-resolutions-and-amend push-notes show-stat subrepo-pin-state switch-says
- In tags
- v0.1.0 v0.10.0 v0.11.0 v0.2.0 v0.3.0 v0.3.1 v0.4.0 v0.4.1 v0.4.2 v0.4.3 v0.4.4 v0.4.5 v0.4.6 v0.5.0 v0.5.1 v0.6.0 v0.7.0 v0.8.0 v0.9.0
- Edits
- 5 atoms, 0 tree operations, 1 file
Browse the tree at this change
modified crates/oxy-core/src/store.rs +80 −4
| … | 80 unchanged lines |
||
| 81 | 81 | .ok_or_else(|| bad("unknown object type"))?, |
|
| 82 | 82 | flags: e[49], |
|
| 83 | 83 | }; |
|
| 84 | - | if entry.stored_len > MAX_OBJECT_LEN |
|
| 85 | - | || entry.raw_len > MAX_OBJECT_LEN |
|
| 86 | - | || entry.offset.saturating_add(entry.stored_len as u64) > pack_len |
|
| 87 | - | { |
|
| 84 | + | if entry.stored_len > MAX_OBJECT_LEN || entry.raw_len > MAX_OBJECT_LEN { |
|
| 85 | + | return Err(StoreError::Corrupt( |
|
| 86 | + | r, |
|
| 87 | + | format!( |
|
| 88 | + | "{}: the index claims an object of {} bytes ({} stored), more than the {MAX_OBJECT_LEN} an object may be", |
|
| 89 | + | idx_path.display(), |
|
| 90 | + | entry.raw_len, |
|
| 91 | + | entry.stored_len |
|
| 92 | + | ), |
|
| 93 | + | )); |
|
| 94 | + | } |
|
| 95 | + | if entry.offset.saturating_add(entry.stored_len as u64) > pack_len { |
|
| 88 | 96 | return Err(bad("index entry points outside the pack")); |
|
| 89 | 97 | } |
|
| 90 | 98 | entries.push(entry); |
|
| … | 66 unchanged lines |
||
| 157 | 165 | WrongType(ObjectRef, &'static str, &'static str), |
|
| 158 | 166 | #[error("object {0} not found")] |
|
| 159 | 167 | NotFound(ObjectRef), |
|
| 168 | + | #[error("a {kind} object of {size} bytes is larger than the {limit} bytes an object may be; nothing was stored. A first commit of a very large tree makes one change holding every text line: commit it in parts (a directory at a time)")] |
|
| 169 | + | TooLarge { |
|
| 170 | + | kind: &'static str, |
|
| 171 | + | size: usize, |
|
| 172 | + | limit: u32, |
|
| 173 | + | }, |
|
| 160 | 174 | } |
|
| 161 | 175 | ||
| 162 | 176 | pub type Result<T> = std::result::Result<T, StoreError>; |
|
| … | 578 unchanged lines |
||
| 741 | 755 | ||
| 742 | 756 | /// Store `body` as an object of type `ty`; idempotent. |
|
| 743 | 757 | pub fn put(&self, ty: ObjectType, body: &[u8]) -> Result<ObjectRef> { |
|
| 758 | + | // Refuse what the store could not read back (spec §13.2), before |
|
| 759 | + | // anything is written: a pack must never hold an entry its index |
|
| 760 | + | // would reject. |
|
| 761 | + | if body.len() > MAX_OBJECT_LEN as usize { |
|
| 762 | + | return Err(StoreError::TooLarge { |
|
| 763 | + | kind: ty.name(), |
|
| 764 | + | size: body.len(), |
|
| 765 | + | limit: MAX_OBJECT_LEN, |
|
| 766 | + | }); |
|
| 767 | + | } |
|
| 744 | 768 | let r = hash_object(ty, body); |
|
| 745 | 769 | let path = self.path_for(&r); |
|
| 746 | 770 | if path.is_file() || self.find_packed(&r).is_some() { |
|
| … | 684 unchanged lines |
||
| 1431 | 1455 | ); |
|
| 1432 | 1456 | } |
|
| 1433 | 1457 | assert!(matches!(got[40], Err(StoreError::NotFound(_)))); |
|
| 1458 | + | } |
|
| 1459 | + | } |
|
| 1460 | + | ||
| 1461 | + | #[cfg(test)] |
|
| 1462 | + | mod batch_tests { |
|
| 1463 | + | use super::*; |
|
| 1464 | + | ||
| 1465 | + | #[test] |
|
| 1466 | + | fn a_large_batch_packs_every_object_and_indexes_them_all() { |
|
| 1467 | + | let dir = tempfile::tempdir().unwrap(); |
|
| 1468 | + | let store = Store::open(dir.path()).unwrap(); |
|
| 1469 | + | let mut refs = Vec::new(); |
|
| 1470 | + | { |
|
| 1471 | + | let batch = store.begin_batch().unwrap(); |
|
| 1472 | + | for i in 0..1000u32 { |
|
| 1473 | + | let body = format!("object {i} {}", "x".repeat(20_000 + i as usize)).into_bytes(); |
|
| 1474 | + | refs.push(store.put(ObjectType::Blob, &body).unwrap()); |
|
| 1475 | + | } |
|
| 1476 | + | batch.finish().unwrap(); |
|
| 1477 | + | } |
|
| 1478 | + | let again = Store::open(dir.path()).unwrap(); |
|
| 1479 | + | for r in &refs { |
|
| 1480 | + | again |
|
| 1481 | + | .get(r) |
|
| 1482 | + | .unwrap_or_else(|e| panic!("{}: {e}", r.short(12))); |
|
| 1483 | + | } |
|
| 1484 | + | } |
|
| 1485 | + | } |
|
| 1486 | + | ||
| 1487 | + | #[cfg(test)] |
|
| 1488 | + | mod size_tests { |
|
| 1489 | + | use super::*; |
|
| 1490 | + | ||
| 1491 | + | #[test] |
|
| 1492 | + | fn an_object_over_the_limit_is_refused_and_nothing_is_written() { |
|
| 1493 | + | let dir = tempfile::tempdir().unwrap(); |
|
| 1494 | + | let store = Store::open(dir.path()).unwrap(); |
|
| 1495 | + | let big = vec![b'x'; MAX_OBJECT_LEN as usize + 1]; |
|
| 1496 | + | { |
|
| 1497 | + | let batch = store.begin_batch().unwrap(); |
|
| 1498 | + | store.put(ObjectType::Blob, b"small").unwrap(); |
|
| 1499 | + | let e = store.put(ObjectType::Change, &big).unwrap_err(); |
|
| 1500 | + | assert!( |
|
| 1501 | + | matches!(e, StoreError::TooLarge { kind: "change", .. }), |
|
| 1502 | + | "{e}" |
|
| 1503 | + | ); |
|
| 1504 | + | assert!(e.to_string().contains("commit it in parts"), "{e}"); |
|
| 1505 | + | batch.finish().unwrap(); |
|
| 1506 | + | } |
|
| 1507 | + | // The store still opens and reads what was written. |
|
| 1508 | + | let again = Store::open(dir.path()).unwrap(); |
|
| 1509 | + | again.get(&hash_object(ObjectType::Blob, b"small")).unwrap(); |
|
| 1434 | 1510 | } |
|
| 1435 | 1511 | } |