Skip to content

posixfs, s3, sftp: URL-unquote#131

Open
ThomasWaldmann wants to merge 2 commits intoborgbackup:masterfrom
ThomasWaldmann:url-unquote
Open

posixfs, s3, sftp: URL-unquote#131
ThomasWaldmann wants to merge 2 commits intoborgbackup:masterfrom
ThomasWaldmann:url-unquote

Conversation

@ThomasWaldmann
Copy link
Member

URL components might be URL-quoted (URL-encoded), thus we need to unquote them if we split the URL into its components.

For example, a space in a path would be encoded as %20.

rclone is different: we get rclone:remote_spec and remote_spec is in the rclone-specific format that we just pass through "as is" to rclone.

@ThomasWaldmann
Copy link
Member Author

@alexandru-bagu @mikemrm @ncw could you please review the URL-unquoting?

Guess some unquote-calls were missing for S3 (and also for file: and sftp:), but for rclone: just pass-through "as is" is correct.

@ThomasWaldmann
Copy link
Member Author

URL components might be URL-quoted (URL-encoded), thus we need to unquote them if we split the URL into its components.

For example, a space in a path would be encoded as %20.

rclone is different: we get rclone:remote_spec and remote_spec is in the rclone-specific format that we just pass through "as is" to rclone.
if m:
return Sftp(username=m["username"], hostname=m["hostname"], port=int(m["port"] or "0"), path=m["path"])
return Sftp(
username=unquote(m["username"]) if m["username"] else m["username"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since (?P<username>[^@]+) always matches at least one character the m["username"] in the else case will always be None. Why not use None directly instead of failing to get the group again?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants