Skip to content

Add file name compression - #4496

Merged
ncw merged 4 commits into
rclone:masterfrom
klauspost:add-filename-compression
Aug 13, 2020
Merged

ncw merged 4 commits into
rclone:masterfrom
klauspost:add-filename-compression

Conversation

@klauspost

@klauspost klauspost commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

What is the purpose of this change?

Allows to compress short arbitrary strings and returns a string using base64 url encoding.

Generator for tables included and a few samples has been added. Add more tables to init.go

Tested with fuzzing for crash resistance and symmetry, see fuzz.go - new tables should not affect this.

Requires klauspost/compress#275 - until release a replace has been added to go.mod

Was the change discussed in an issue or in the forum before?

Yes. Twitter.

Checklist

  • I have read the contribution guidelines.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • I'm done, this Pull Request is ready for review :-)
  • Merge upstream PR and update go.mod.

Allows to compress short arbitrary strings and returns a string using base64 url encoding.

Generator for tables included and a few samples has been added.
Add more to init.go

Tested with fuzzing for crash resistance and symmetry, see fuzz.go

Requires klauspost/compress#275 - until release a replace has been added to go.mod
@klauspost
klauspost force-pushed the add-filename-compression branch from 4005cbf to 570e052 Compare August 10, 2020 20:27
@klauspost
klauspost requested a review from ncw August 10, 2020 20:28
@klauspost

klauspost commented Aug 10, 2020

Copy link
Copy Markdown
Contributor Author

The replace will make CI fail.

Fuzz corpus: testdata.zip - didn't want to add 370 files to repo.

@klauspost

Copy link
Copy Markdown
Contributor Author

The change is merged to master. Will be released as v1.10.11 eventually - replacing it with master for now.

When you have added tables adding regression tests for a bunch of strings should be trivial.

@ncw ncw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That is brilliant, thank you Klaus :-)

I'll merge that now.

I'll probably need to change the interface a bit when I glue it onto crypt, but that can wait for the moment.

PS The test failure was a heisenbug - I've nearly killed them all but there are a few left!

@ncw
ncw merged commit cb7534d into rclone:master Aug 13, 2020
@klauspost
klauspost deleted the add-filename-compression branch August 13, 2020 16:10
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