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

Fix GitHub Actions Cache setting an incorrect Content-Range header #1572

Merged

Conversation

j-mie
Copy link
Contributor

@j-mie j-mie commented Jan 15, 2025

I recently tried to use the VCPKG_BINARY_SOURCES setting to set clear;x-gha,readwrite in our GitHub Actions workflow and couldn't get vcpkg to upload to the cache successfully. Proxying vcpkg I noticed that the Content-Range header was off by one causing the cache server to return:

HTTP/2 400 Bad Request
Content-Length: 295
Content-Type: application/json; charset=utf-8
Date: Wed, 15 Jan 2025 02:35:10 GMT
Server: Kestrel
Cache-Control: no-store,no-cache
Pragma: no-cache
...
X-Frame-Options: SAMEORIGIN

{"$id":"1","innerException":null,"message":"The size of the content did not match the Content-Range header","typeName":"Microsoft.Azure.DevOps.ArtifactCache.WebApi.InvalidChunkException, Microsoft.Azure.DevOps.ArtifactCache.WebApi","typeKey":"InvalidChunkException","errorCode":0,"eventId":3000}

Manually correcting the value and then sealing the cache with the POST request containing the size field resulted in the package being correctly cached. I'm not sure why users of the GitHub Actions binary caching feature in vcpkg haven't run into this on github.com and haven't had the time to see if github.com handles this case differently but it does work on GitHub Enterprise Server.

@@ -930,7 +930,7 @@ namespace
m_token_header,
m_accept_header.to_string(),
"Content-Type: application/octet-stream",
"Content-Range: bytes 0-" + std::to_string(cache_size) + "/*",
"Content-Range: bytes 0-" + std::to_string(cache_size - 1) + "/*",
Copy link
Member

@BillyONeal BillyONeal Jan 15, 2025

Choose a reason for hiding this comment

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

Needs to be guarded for cache_size == 0. Probably by another if around the if on line 927, or an early return.

Also it seems like it would be a good idea to fill in the size based on https://developer.mozilla.org/docs/Web/HTTP/Headers/Content-Range ?

Suggested change
"Content-Range: bytes 0-" + std::to_string(cache_size - 1) + "/*",
fmt::format("Content-Range: bytes 0-{}/{}", cache_size - 1, cache_size),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix that now

Regarding the size, the enterprise backend doesn't seem to mind if you set it or not, I think it's probably more relevant when using chunking as in #1043 but will also fix.

@j-mie j-mie force-pushed the jamieh/fix-ghes-binary-caching branch 3 times, most recently from b16772d to 872a3e9 Compare January 15, 2025 21:18
Content-Range is 0 indexed, although nobody seemed to have encountered
any issues as a result of this on github.com, but on GitHub Enterprise
server it resulted in a InvalidChunkException error
@j-mie j-mie force-pushed the jamieh/fix-ghes-binary-caching branch from 872a3e9 to 11e1527 Compare January 15, 2025 21:21
@BillyONeal BillyONeal merged commit 52e8ac9 into microsoft:main Jan 15, 2025
6 checks passed
@BillyONeal
Copy link
Member

Thanks for the fix!

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