feat: set Content-MD5 header to avoid silent data corruption by bhearsum · Pull Request #815 · mozilla-releng/scriptworker · GitHub
Skip to content

feat: set Content-MD5 header to avoid silent data corruption - #815

Merged
bhearsum merged 1 commit into
mozilla-releng:mainfrom
bhearsum:feat-set-content-md5-header-to
Aug 11, 2026
Merged

bhearsum merged 1 commit into
mozilla-releng:mainfrom
bhearsum:feat-set-content-md5-header-to

Conversation

@bhearsum

@bhearsum bhearsum commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

In bug 2060889 we're looking at some chain of trust verification failures caused by hash mismatches between chain-of-trust.json and the files in GCS. After a great deal of debugging, I ended up finding that internal aiohttp retries on PUT requests can send incomplete data. The short version of that bug is that the file handle is advanced every time a request is prepared, and if a failure+retry happens after that, it advances to the next chunk of data rather than resending the failed chunk.

GCS uploads support the Content-MD5 header, which will cause a 400 error to be returned if the uploaded data does not match the md5 provided in the header. Setting this will let us retry at our level if an upload is corrupt in this specific scenario, but more importantly, for any other future scenario that may come up. This means that we don't benefit from aiohttp's retries, which might save a bit of bandwidth and time, but it's a more general solution.

A different way to work around this would be avoid passing plain file handles and either pass the data (impractical due to the size of the files), or one of aiohttp's Payload objects. The latter seems to work, although they're not well documented, and I don't understand it enough to feel confident in doing so.

In [bug 2060889](https://bugzilla.mozilla.org/show_bug.cgi?id=2060889) we're looking at some chain of trust verification failures caused by hash mismatches between `chain-of-trust.json` and the files in GCS. After a great deal of debugging, I ended up finding that [internal aiohttp retries on PUT requests can send incomplete data](aio-libs/aiohttp#13329). The short version of that bug is that the file handle is advanced every time a request is prepared, and if a failure+retry happens after that, it advances to the next chunk of data rather than resending the failed chunk.

GCS uploads [support the Content-MD5 header](https://docs.cloud.google.com/storage/docs/data-validation#xml-single-request-upload), which will cause a 400 error to be returned if the uploaded data does not match the md5 provided in the header. Setting this will let us retry at our level if an upload is corrupt in this specific scenario, but more importantly, for any other future scenario that may come up. This means that we don't benefit from aiohttp's retries, which might save a bit of bandwidth and time, but it's a more general solution.

A different way to work around this would be avoid passing plain file handles and either pass the data (impractical due to the size of the files), or one of aiohttp's Payload objects. The latter seems to work, although they're not well documented, and I don't understand it enough to feel confident in doing so.
@bhearsum
bhearsum marked this pull request as ready for review August 5, 2026 18:15
@bhearsum
bhearsum requested a review from a team as a code owner August 5, 2026 18:15

@ahal ahal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds reasonable!

Seems like a pretty bad bug in aiohttp though, which is surprising. Maybe we're not using an up-to-date version.

@bhearsum

bhearsum commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@bhearsum
bhearsum merged commit ce4070c into mozilla-releng:main Aug 11, 2026
12 checks passed
bhearsum added a commit that referenced this pull request Aug 12, 2026
This is causing permafailures:
```
public/logs/chain_of_trust.log was corrupted in transit: the storage backend rejected the body as not matching Content-MD5 +r5pkJPuO/JWdjABXz6gEQ==. Retrying.
```

I'm guessing the md5 calculation is mismatched somehow. It's probably not worth debugging considering aiohttp merged a fix upstream (aio-libs/aiohttp#13330), and the issue this was attempting to work around is so rare.
bhearsum added a commit that referenced this pull request Aug 12, 2026
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.

2 participants