Add the file hash to the update link answer (bug 1158738) #527
Conversation
magopian
force-pushed
the
1158738-add-file-hash-to-update-file-link
branch
from
f82edd5
to
f15338e
Compare
| return posixpath.join(host, str(id), row['filename']) | ||
| url = posixpath.join(host, str(id), row['filename']) | ||
| params = urllib.urlencode({'filehash': row['hash']}) | ||
| return '{0}?{1}'.format(url, params) |
There was a problem hiding this comment.
Is this just for cache busting? Or also for security checks? For cache busting it seems heavy to include the full sha and prefix? It's fine, computers don't mind, but the URL ends up being quite ugly.
There was a problem hiding this comment.
Only for cache busting, the security is done elsewhere. What would you recommend?
There was a problem hiding this comment.
You could truncate the hash at 8 characters
There was a problem hiding this comment.
I have absolutely nothing against that, but what would be the benefit? It's not a public facing url, it's the one that's answered to the client (Firefox) on automatic version update checks.
There was a problem hiding this comment.
That's true I guess. Ok, computers don't mind ugly URLs.
|
r+ |
…-file-link Add the file hash to the update link answer (bug 1158738)
Fixes bug 1158738
I tried to keep the code as simple and straightforward as possible, as this service is critical and needs a high throughput. That's also why I didn't factorize with https://github.com/mozilla/olympia/pull/512/files#diff-833bbde9ea94ca5d1320b5b7e98b9994R105, and why I didn't use
amo.utils.urlparams(we don't want to import that in this file).