-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Base64 encode the scope value #51
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Welcome to Rust 😄
I'd appreciate if you could add test coverage as well; you should be able to duplicate this test with a scope that breaks prior to this PR.
Thanks for the feedback. I'll try to take care of these sometime this week. |
This commit adds a test that exercises the issue described in dimo414#50.
bkt permits setting a cache scope (provided via the `BKT_SCOPE` environment variable or the `--scope` command line option) to prevent collisions between unrelated command invocations. Previously, the scope value was used verbatim in a directory path, causing errors if the scope contained path-sensitive characters such as `/`. This commit modifies bkt to base64 encode the scope value. Given a scope value like `https://example.com/`, that means we now end up with a path like: /tmp/bkt-0.7-cache/keys/aHR0cHM6Ly9leGFtcGxlLmNvbS8.C3823B193F1E0C78 Instead of the invalid: /tmp/bkt-0.7-cache/keys/https://www.example.com/.C3823B193F1E0C78 Fixes dimo414#50
@dimo414 just following up to see if this needs any more work. Thanks! |
Sorry I had missed that you'd updated the PR (you can click the "re-request review" button to notify the reviewer - GitHub's review flow is pretty confusing 😕). Will take a look as soon as I can. |
Thanks again! Will plan to cut a new release with this soon. |
bkt permits setting a cache scope (provided via the
BKT_SCOPE
environment variable or the
--scope
command line option) to preventcollisions between unrelated command invocations. Previously, the scope
value was used verbatim in a directory path, causing errors if the scope
contained path-sensitive characters such as
/
.This commit modifies bkt to base64 encode the scope value. Given a scope
value like
https://example.com/
, that means we now end up with a pathlike:
Instead of the invalid:
Fixes #50