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 fnv1a instead of SHA256 to create images signatures #2353

Merged
merged 7 commits into from
Jun 24, 2024

Conversation

jpreynat
Copy link
Member

Because we use a private key alongside the URLs to sign server side, using a simpler and non-cryptographic hashing function such as fnv1a that can't be reversed should be secure enough.
It should eventually yield to way faster rendering on pages that have many images because we potentially compute these hashes for each image, and it will also make the resize endpoint faster too, since we need to recompute the hash there.

Copy link

argos-ci bot commented Jun 21, 2024

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 4 changed Jun 24, 2024, 11:24 AM

src/lib/images.ts Outdated Show resolved Hide resolved
Copy link
Member

@SamyPesse SamyPesse left a comment

Choose a reason for hiding this comment

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

We can't just ship this as it'll break all images at the moment. We need a compatibility layer

@jpreynat
Copy link
Member Author

@SamyPesse Done in 2a7104e

Copy link
Member

@SamyPesse SamyPesse left a comment

Choose a reason for hiding this comment

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

It looks good, but we should test urls on preview url to ensure previous urls still work

src/lib/images.ts Outdated Show resolved Hide resolved
@jpreynat
Copy link
Member Author

jpreynat commented Jun 24, 2024

Testing old images URLs using the previous algorithm from another deployment:

https://676a716a.gitbook-open.pages.dev/~gitbook/image?url=https%3A%2F%2F1050631731-files.gitbook.io%2F%7E%2Ffiles%2Fv0%2Fb%2Fgitbook-x-prod.appspot.com%2Fo%2Fspaces%252FNkEGS7hzeqa35sMXQZ4X%252Fuploads%252FYHeUUs1j7fxDUv8xxWVW%252Fcontent-editor.png%3Falt%3Dmedia%26token%3D992c96ed-1cf3-43da-b4c0-d7ca8bb2ed53&width=245&dpr=4&quality=100&sign=70577215f4d016f554c2232e4708a05a7fe32786e34f3094696c35d7b5c99071

Using a deployment from this branch with the old algorithm:

https://f7da6398.gitbook-open.pages.dev/~gitbook/image?url=https%3A%2F%2F1050631731-files.gitbook.io%2F%7E%2Ffiles%2Fv0%2Fb%2Fgitbook-x-prod.appspot.com%2Fo%2Fspaces%252FNkEGS7hzeqa35sMXQZ4X%252Fuploads%252FYHeUUs1j7fxDUv8xxWVW%252Fcontent-editor.png%3Falt%3Dmedia%26token%3D992c96ed-1cf3-43da-b4c0-d7ca8bb2ed53&width=245&dpr=4&quality=100&sign=70577215f4d016f554c2232e4708a05a7fe32786e34f3094696c35d7b5c99071

New URL from this branch (new algorithm):

https://f7da6398.gitbook-open.pages.dev/~gitbook/image?url=https%3A%2F%2F1050631731-files.gitbook.io%2F%7E%2Ffiles%2Fv0%2Fb%2Fgitbook-x-prod.appspot.com%2Fo%2Fspaces%252FNkEGS7hzeqa35sMXQZ4X%252Fuploads%252FYHeUUs1j7fxDUv8xxWVW%252Fcontent-editor.png%3Falt%3Dmedia%26token%3D992c96ed-1cf3-43da-b4c0-d7ca8bb2ed53&width=245&dpr=4&quality=100&sign=54cd6950&sv=1

@jpreynat jpreynat merged commit e3f5f81 into main Jun 24, 2024
7 of 8 checks passed
@jpreynat jpreynat deleted the faster-images-signature branch June 24, 2024 11:31
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.

3 participants