Skip to content

Trim double quotes from received ETag when checking hashes#175

Merged
ncw merged 1 commit into
ncw:masterfrom
timss:trim-quoted-etag
Oct 6, 2021
Merged

Trim double quotes from received ETag when checking hashes#175
ncw merged 1 commit into
ncw:masterfrom
timss:trim-quoted-etag

Conversation

@timss

@timss timss commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

This issue was originally discussing over at the rclone forums:
https://forum.rclone.org/t/intended-behavior-of-sync-and-checksum-for-swift/26743


For Swift servers/accounts/containers that are configured to use RFC-compliant ETags enclosed in double quotes[1][2], rclone was mistakenly considering objects corrupted when comparing "<hash>" vs <hash>.

This could happen for operations such as sync (both ways), typically during transfer (ETag in response to PUT request`), or possibly triggering retransfers when object already present at destination (comparing object checksum in JSON formatted container listing vs. header ETag).

$ rclone sync --config rclone.conf src:container dest:container\
  --progress --checksum -vv --dump requests,responses
[...]
2021-09-30 11:38:18 DEBUG : myfile.gz: md5 = <hash> (Swift container mycontainer>
2021-09-30 11:38:18 DEBUG : myfile.gz: md5 = "<hash>" (Swift container mycontainer>
2021-09-30 11:38:18 DEBUG : md5 differ

I have tested my patch on Swift v2.25.1 together with rclone v1.56.2 on Go 1.17.1.
This is my first time playing around with Go so if there's anything that needs to be addressed please let me know :-)

[1] https://github.com/openstack/swift/blob/2.24.0/CHANGELOG#L9
[2] https://datatracker.ietf.org/doc/html/rfc7232#section-2.3

If a Swift server is configured to adhere to RFC-compliant ETags,
either by default for all accounts, or opt-in per account/container,
ETags may be double quoted.

https://github.com/openstack/swift/blob/2.24.0/CHANGELOG#L9

@ncw ncw left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

That looks great, thanks.

I'll merge it now.

@ncw ncw merged commit ee993b7 into ncw:master Oct 6, 2021
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