Strengthen the hashing to prevent hash collisions.
As a follow-up to #2844852: Minify the cache tags sent in the header, hash collisions happen.
#2844852-11: Minify the cache tags sent in the header shows this example:
node_list and node:287766
Both have a hash of ef74.
Proposal is to convert the base from 16 to 64, and use base64url encoding.
Currently 4 characters base 16 is used = 65536 unique hashes.
If we switch to 4 characters base 64 = 16777216 unique hashes.
I have grabbed some base conversion code from the internet and drafted the code here to validate my assumptions.
Here's the test: http://sandbox.onlinephpfunctions.com/code/ad1431ad58d0e740fa7e10e1259d1...
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | interdiff_3_4.txt | 1.14 KB | miroslavbanov |
| #4 | stronger_hash-3033502-4.patch | 1.05 KB | miroslavbanov |
| #3 | stronger_hash-3033502-3.patch | 854 bytes | miroslavbanov |
Comments
Comment #2
miroslavbanov commentedRemoved redundant description.
Some materials on base conversion:
http://elenzil.com/esoterica/baseConversion.html
https://stackoverflow.com/a/4964352
Comment #3
miroslavbanov commentedAdding a patch with only base32 encoding.
Unfortunately I believe that Acquia VCL lowercases the string so base64 probably isn't going to collide less until that is changed.
Comment #4
miroslavbanov commentedAdding the base64 version.
Like I said above, unfortunately I believe that Acquia VCL lowercases the string so base64 probably isn't going to collide less than base32 until that is changed.
Comment #5
nielsvm commentedThank you for your amazingly helpful contribution MiroslavBanov, I've tested it extensively. You're correct on assuming that we lowercase the strings here at Acquia so we're indeed going for the B32 version as we're reluctant about making our server-side implementation case sensitive (and we're not sure that's possible/safe).
This got included in the recent release, thanks everyone!
https://www.drupal.org/project/acquia_purge/releases/8.x-1.0
Comment #6
josh waihi commentedThis breaks invalidation of existing cached pages right? So Varnish will need to be purged with the release of 8.x-1.0?
Comment #7
nielsvm commentedHey Josh - Yes, and this is in the release notes :-).
This was one of the changes that I only wanted to make between beta-releases, so going forward the idea is that new releases don't break existing installations in the way the 1.0 does. Worst case we could write update-hooks in the future, although those aren't perfect either.