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

MAX_CHUNK_HEADER_SIZE = 4096 is too small for podman #3362

Closed
ianballou opened this issue Apr 4, 2024 · 6 comments
Closed

MAX_CHUNK_HEADER_SIZE = 4096 is too small for podman #3362

ianballou opened this issue Apr 4, 2024 · 6 comments

Comments

@ianballou
Copy link

Describe the bug
When podman tries to push container content in a chunked fashion to our container registry that uses Puma, the chunk.size here

if @prev_chunk.size + chunk.size >= MAX_CHUNK_HEADER_SIZE
is sometimes exactly 4096. That triggers raise HttpParserError, "maximum size of chunk header exceeded" because @prev_chunk.size + chunk.size >= MAX_CHUNK_HEADER_SIZE, and MAX_CHUNK_HEADER_SIZE is 4096.

I'm curious why 4096 was recently chosen as the MAX_CHUNK_HEADER_SIZE, and if it's something that should be made more easily customizable.

If I change the MAX_CHUNK_HEADER_SIZE to something like 4100, the issue goes away entirely.

@MSP-Greg
Copy link
Member

MSP-Greg commented Apr 5, 2024

is sometimes exactly 4096

Would changing the line you referenced so that it used > instead of >= solve your issue?

@ianballou
Copy link
Author

is sometimes exactly 4096

Would changing the line you referenced so that it used > instead of >= solve your issue?

I'm still learning how podman does its chunked uploads, but I've noticed that the previous chunk size can vary, so even with >= I'll probably end up exceeding 4096 (since the previous chunk size and the current chunk size get added together in the puma code).

That's why I tried hacking the size to 4100, which seemed to solve the problem in one particular case.

I'm not sure if podman is doing something strange by having a chunked upload size of 4096, I was hoping get a better idea about that by finding out where the puma limit of 4096 came from.

@MSP-Greg
Copy link
Member

MSP-Greg commented Apr 5, 2024

Sorry, I was thinking that I've looked at this previously. I looked around...

Can you try #3338?

@ianballou
Copy link
Author

Can you try #3338?

It looks like this worked! I can't speak to the correctness, but my podman push no longer errors out :)

@ianballou
Copy link
Author

Now that ebb40b6 is merged, that should solve this issue. How soon might we be able to expect a release with the fix?

@dentarg
Copy link
Member

dentarg commented Apr 18, 2024

It's up to Nate. The goal is every ~4 weeks but that was obviously a while ago. I'm sure it will happen when Nate has time for it. Closing this as a duplicate.

@dentarg dentarg closed this as completed Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants