don't mistake a malformed URL for a local path, #10215 - #10216
Open
ThomasWaldmann wants to merge 1 commit into
Open
don't mistake a malformed URL for a local path, #10215#10216ThomasWaldmann wants to merge 1 commit into
ThomasWaldmann wants to merge 1 commit into
Conversation
A "rest://host/" URL (no repository path) did not match rest_re, fell through to local_re and became the local directory "./rest:/host", which then failed with a confusing "Repository ... does not exist" message. Only "ssh://" and "file://" were excluded from local_re, "rest://" was not. Exclude all schemes borg parses itself from local_re, in the opaque form too, so no "rest:...", "ssh:..." or "file:..." input can silently be taken for a local path, and tell the accepted forms when such a URL does not parse. A local path that really starts with such a prefix can still be given as "./rest:..." or as an absolute path.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10216 +/- ##
==========================================
- Coverage 87.29% 87.29% -0.01%
==========================================
Files 102 102
Lines 18444 18451 +7
Branches 2834 2834
==========================================
+ Hits 16101 16107 +6
Misses 1638 1638
- Partials 705 706 +1 ☔ View full report in Codecov by Harness. |
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 the confusing behaviour reported in discussion #10215:
borg -r rest://dcm/ repo-infofailed withCause
rest://dcm/carries no repository path, so it does not matchrest_re(which needs at least onecharacter after the separating slash). It then fell through to
local_re, whose negative lookaheadexcluded
ssh://,file://and the borgstore schemes, but notrest://. So the URL was takenfor a relative local path and prefixed with the cwd.
Change
All schemes borg parses itself (
ssh,rest,file) are now excluded fromlocal_re, in theopaque
scheme:form as well as in thescheme://form. Nothing starting with those prefixes cansilently become a local path any more - such input is rejected instead. A local path that really
starts with such a prefix can still be given as
./rest:...or as an absolute path (there is a testfor that).
The reporter said they had tried numerous permutations without getting any useful feedback, so the
error now also names the accepted forms:
Docs
quickstart.rstrecommendedrest://borg@remote_host(no path), which would have run intoexactly this error.
for the remote protocols, but it is relative to the remote login directory. Corrected for
rest/ssh/sftp, and added a note that the path is required and what one vs. two slashes mean.
Tests
New tests cover the malformed
rest://forms, the hint per scheme, local paths starting with ascheme name, and
rest://canonical-path round trips. Full test suite passes locally.