borg check aborts with an unhandled ValueError: Odd-length string and a full traceback if packs/ contains an object whose name is not a valid hex pack id. The stray object is never reported, and no pack after it is checked.
Found while reviewing #9925; it is not caused by that branch, it reproduces on current master (423bc3117).
Reproduce
borg repo-create -e aes256-ocb
borg create test /some/path
touch "$BORG_REPO"/packs/*/not-a-pack-name
borg check --repository-only
Starting full repository check
Starting from beginning.
Local Exception
Error:
ValueError: Odd-length string
...
File "src/borg/helpers/parseformat.py", line 58, in hex_to_bin
raise ValueError(str(e)) from None
ValueError: Odd-length string
Exit code 2, and the traceback-with-please-report-this banner.
Cause
Repository.check() decodes every name returned by the packs/ listing without validating it:
https://github.com/borgbackup/borg/blob/423bc3117/src/borg/repository.py#L1024
for info in pack_infos:
...
pack_id = hex_to_bin(info.name)Any name that is not an even-length hex string raises out of hex_to_bin and unwinds the whole command. Because the listing is walked in name order, packs sorting after the stray name are never verified.
index/ does not have this problem: verify("index", info.name) only compares store.hash(key) against the name, so a stray object there is cleanly reported as Store object index/xxx is corrupted: content does not match its name (sha256). and counted as an error. packs/ should behave the same way.
This is the only place in repository.py that decodes a name coming from a store listing — compact works from the index, not from a listing — so the fix is local.
How likely is this in practice?
Low, but not impossible. borgstore rejects most junk at store time (uppercase, blanks, .., \, over-long names — so e.g. .DS_Store cannot be created through the API), and its list() deliberately skips TMP_SUFFIX entries, so an interrupted upload leaves nothing that reaches this code. What remains is anything that got into packs/ outside borgstore: a lowercase stray file from an operator, a restore/rsync that brought along an extra file, a filesystem or sync tool leaving a lowercase artifact.
The reason it seems worth fixing anyway is that borg check is the command you reach for when you already suspect something is wrong with a repository, and this failure mode gives a traceback rather than the name of the offending object.
Suggested fix
Validate the listing name before decoding, and report a bad one as an error instead of raising — something like:
if not is_valid_pack_name(info.name): # 64 lowercase hex chars
logger.error(f"Store object packs/{info.name} is not a valid pack name; ignoring it.")
pack_errors += 1
continueThat keeps the scan going, names the offending object, and makes the check fail (exit 2 as a warning) for a reason the user can act on. Whether it should count as an error or a warning is a judgement call — reporting it and continuing is the important part.
borg checkaborts with an unhandledValueError: Odd-length stringand a full traceback ifpacks/contains an object whose name is not a valid hex pack id. The stray object is never reported, and no pack after it is checked.Found while reviewing #9925; it is not caused by that branch, it reproduces on current master (
423bc3117).Reproduce
Exit code 2, and the traceback-with-please-report-this banner.
Cause
Repository.check()decodes every name returned by thepacks/listing without validating it:https://github.com/borgbackup/borg/blob/423bc3117/src/borg/repository.py#L1024
Any name that is not an even-length hex string raises out of
hex_to_binand unwinds the whole command. Because the listing is walked in name order, packs sorting after the stray name are never verified.index/does not have this problem:verify("index", info.name)only comparesstore.hash(key)against the name, so a stray object there is cleanly reported asStore object index/xxx is corrupted: content does not match its name (sha256).and counted as an error.packs/should behave the same way.This is the only place in
repository.pythat decodes a name coming from a store listing — compact works from the index, not from a listing — so the fix is local.How likely is this in practice?
Low, but not impossible. borgstore rejects most junk at store time (uppercase, blanks,
..,\, over-long names — so e.g..DS_Storecannot be created through the API), and itslist()deliberately skipsTMP_SUFFIXentries, so an interrupted upload leaves nothing that reaches this code. What remains is anything that got intopacks/outside borgstore: a lowercase stray file from an operator, a restore/rsync that brought along an extra file, a filesystem or sync tool leaving a lowercase artifact.The reason it seems worth fixing anyway is that
borg checkis the command you reach for when you already suspect something is wrong with a repository, and this failure mode gives a traceback rather than the name of the offending object.Suggested fix
Validate the listing name before decoding, and report a bad one as an error instead of raising — something like:
That keeps the scan going, names the offending object, and makes the check fail (exit 2 as a warning) for a reason the user can act on. Whether it should count as an error or a warning is a judgement call — reporting it and continuing is the important part.