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

Found bug in migration checksum #1887

Closed
bryanmcgrane opened this issue May 2, 2021 · 2 comments · Fixed by #1889
Closed

Found bug in migration checksum #1887

bryanmcgrane opened this issue May 2, 2021 · 2 comments · Fixed by #1889
Assignees
Labels
team/schema Issue for team Schema. tech/engines/migration engine Issue in the Migration Engine
Milestone

Comments

@bryanmcgrane
Copy link

bryanmcgrane commented May 2, 2021

I noticed there is an issue with the checksum generation code found here:

/// Compute the checksum for a migration script, and return it formatted to be human-readable.

I noticed the issue when inspecting the _prismia_migration tables that are generated in my database.

It appears there is an issue with converting from the bytes to a hex string, where 0's are sometimes being stripped out of the final checksum string. It's relatively simple to replicate the issue. Below is an example (apologies in advance, I'm not a Rust developer):

/// Prisma checksum
checksum("hello")

/// Result is 2cf24dba5fb0a3e26e83b2ac5b9e29e1b161e5c1fa7425e7343362938b9824
/// Note the length is only 62 characters
#!/bin/bash
echo -n hello | shasum -a 256

# Result is 2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824
# Note the length is the correct 64 characters

Characters that are missing from Prisma's checksum
2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824

Edit:
I dug a little deeper and I found the issue was with this line where bytes are converted to hex strings.

write!(checksum_string, "{:x}", byte).unwrap();

write!(checksum_string, "{:x}", byte).unwrap(); needs to have a 0 padding built in for bytes with leading 0's.

@tomhoule tomhoule self-assigned this May 3, 2021
@tomhoule tomhoule added the tech/engines/migration engine Issue in the Migration Engine label May 3, 2021
@tomhoule tomhoule added this to the 2.22.0 milestone May 3, 2021
@tomhoule
Copy link
Contributor

tomhoule commented May 3, 2021

Hi @bryanmcgrane! Good catch, thanks for reporting this, I'll look into it asap. We'll have to be careful to still accept the bad checksums for now, unfortunately (while fixing the new ones), or it will break existing users.

@tomhoule tomhoule added process/candidate team/schema Issue for team Schema. labels May 3, 2021
tomhoule added a commit that referenced this issue May 3, 2021
This is to align with other tools, for example the CLI shasum utility on
linux.

The old format is still recognized and accepted for compatibility with
existing migration tables.

closes #1887
@tomhoule
Copy link
Contributor

tomhoule commented May 3, 2021

PR is here: #1889

I think we should merge after next release (Tuesday), since current checksums aren't completely broken, and that will give us some time to test before releasing that change to users.

tomhoule added a commit that referenced this issue May 3, 2021
This is to align with other tools, for example the CLI shasum utility on
linux.

The old format is still recognized and accepted for compatibility with
existing migration tables.

closes #1887
tomhoule added a commit that referenced this issue May 4, 2021
This is to align with other tools, for example the CLI shasum utility on
linux.

The old format is still recognized and accepted for compatibility with
existing migration tables.

closes #1887
@Jolg42 Jolg42 modified the milestones: 2.22.0, 2.23.0 May 5, 2021
tomhoule added a commit that referenced this issue May 5, 2021
This is to align with other tools, for example the CLI shasum utility on
linux.

The old format is still recognized and accepted for compatibility with
existing migration tables.

closes #1887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team/schema Issue for team Schema. tech/engines/migration engine Issue in the Migration Engine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants