Skip to content
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

Use SHA-1 for non-sensitive digests by default #31651

Merged
merged 1 commit into from Jan 12, 2018

Conversation

@eugeneius
Copy link
Member

@eugeneius eugeneius commented Jan 8, 2018

Followup to #31289.

Instead of providing a configuration option to set the hash function, switch to SHA-1 for new apps and allow upgrading apps to opt in later via new_framework_defaults_5_2.rb.

@georgeclaghorn, does this address your concern from #31289 about cache invalidations for existing apps? Upgrading to 5.2 wouldn't change anything, but flipping the setting later would still invalidate every key. If busting the cache is a complete no-go, you could just never do that.

Instead of providing a configuration option to set the hash function,
switch to SHA-1 for new apps and allow upgrading apps to opt in later
via `new_framework_defaults_5_2.rb`.
@rails-bot
Copy link

@rails-bot rails-bot commented Jan 8, 2018

r? @sgrif

(@rails-bot has picked a reviewer for you, use r? to override)

@eugeneius eugeneius force-pushed the eugeneius:use_sha1_digests branch to d034f48 Jan 8, 2018
@sgrif sgrif merged commit f8afb51 into rails:master Jan 12, 2018
2 checks passed
2 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -672,6 +672,8 @@ There are a few configuration options available in Active Support:

* `config.active_support.time_precision` sets the precision of JSON encoded time values. Defaults to `3`.

* `config.active_support.use_sha1_digests` specifies whether to use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header. Defaults to false.

This comment has been minimized.

@ghiculescu

ghiculescu May 21, 2020
Contributor

i think this defaults to true. #39386

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.