Repository navigation
Classify recovery errors by type, so storage cannot make a valid head look invalid - #20
Merged
Merged
Conversation
… look invalid Recovery decided whether a head was invalid, unavailable, unsupported or unverifiable by matching error message text, and counted a head as unavailable only when an io::Error was in the chain. The carrier backend reported a missing object as a plain string, so a valid head that was only temporarily unreadable was classified Invalid and recover --publish could overwrite it, which the spec forbids. The same mislabelling made the candidate search skip newer valid history. Message matching also let storage-controlled text (for example an unknown field name) change the class. Errors that recovery distinguishes now have types in src/error.rs (ObjectUnavailable, MissingStorageHead, Unsupported, NotActiveReader, ForeignRepository, BudgetExhausted), raised where those conditions occur, and classify_recovery_error inspects only the types in the error chain. read_history_material keeps the original error instead of replacing it with a new string.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes A2 and C6 from the implementation review.
Bugs
classify_recovery_errorcounted a head as Unavailable only if anio::Errorwas in the chain. The carrier backend reports a missing object with a plainbail!, so a valid head that was only temporarily unreadable was classified Invalid, andrecover --publishcould overwrite it. The spec says an Unavailable head must never be overridden. The candidate search also skipped newer valid history for the same reason, becauseread_history_materialreplaced the typed read error with a new string.contains("unsupported"). The text can include storage-controlled content, for example an unknown field name, so storage could steer the class.Reproduced on
mainbefore the fix:recover --publishpublished over it from the floor;unsupportedwas classified Unsupported.Fix
src/error.rswith typed errors:ObjectUnavailable,MissingStorageHead,Unsupported,NotActiveReader,ForeignRepository,BudgetExhausted. They are raised where those conditions occur, in both backends'open_objectand in the manifest, policy, key-file, budget and checkpoint checks. Message text is unchanged.classify_recovery_errorchecks only the error types in the chain, in the same order as before. A non-InvalidDataio::Errorstill counts as Unavailable.read_history_materialkeeps the original error and adds context.contains("budget exhausted")checks inrecoverbecame type checks."cannot open the generation key"branch is removed: no code inmainproduces that message.Tests
carrier_recovery_classifies_missing_head_manifest_as_unavailablecarrier_recovery_refuses_to_skip_a_historical_head_with_missing_objectsrecovery_classification_ignores_storage_controlled_message_text: every old trigger string comes out Invalid.recovery_classification_follows_error_types_through_contextcargo fmt --check,cargo clippy --all-targets -- -D warnings,cargo test --all-targets(144 passed).Carrier chunk-path failures and directory read-time I/O errors are classified exactly as before. Widening what counts as unavailable is a separate decision.