client.rb - fix request chunked body handling

[MSP-Greg]

Description

In client.rb code for processing chunked request bodies, chunk.size was used. This may contain both portions of the body and also hex size data. Hex size data may also contain 'headers'. Use line.size, as it only contains hex size data.

The first commit contains a new test (test_chunked_body_pause_within_chunk_size_hex), which fails on master with the below. Note that there are several test_chunked_body_pause tests, this adds another where a pause exists within the hex encoded length of the next chunk. Thanks to @skliew, who opened issue #3337.

  1) Failure:
TestPumaServer#test_chunked_body_pause_within_chunk_size_hex [test/test_puma_server.rb:1168]:
--- expected
+++ actual
@@ -1,5 +1,15 @@
-"HTTP/1.1 200 OK\r
+# encoding: ASCII-8BIT
+#    valid: true
+"HTTP/1.1 400 Bad Request\r
 Connection: close\r
-Content-Length: 0\r
+Content-Length: 736\r
 \r
-"
+Puma caught this error: maximum size of chunk header exceeded (Puma::HttpParserError)
+../puma/lib/puma/client.rb:646:in 'Puma::Client#decode_chunk'
+../puma/lib/puma/client.rb:530:in 'Puma::Client#setup_chunked_body'
+../puma/lib/puma/client.rb:382:in 'Puma::Client#setup_body'
+../puma/lib/puma/client.rb:275:in 'Puma::Client#try_to_finish'
+../puma/lib/puma/client.rb:287:in 'Puma::Client#eagerly_finish'
+../puma/lib/puma/server.rb:450:in 'Puma::Server#process_client'
+../puma/lib/puma/server.rb:246:in 'block in Puma::Server#run'
+../puma/lib/puma/thread_pool.rb:155:in 'block in Puma::ThreadPool#spawn_thread'"

Closes #3337

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
• My pull request is 100 lines added/removed or less so that it can be easily reviewed.
• If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
• If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
• I have updated the documentation accordingly.
• All new and existing tests passed, including Rubocop.

[MSP-Greg]

Added links to this PR and the related issue to the new test.

[ianballou]

This appears to fix my issue here: #3362

After applying the patch, I'm able to podman push to Foreman's/Katello's/Satellite's container registry without exceeding the max chunk size.

[qcjames53]

This appears to fix my issue here: #3362

After applying the patch, I'm able to podman push to Foreman's/Katello's/Satellite's container registry without exceeding the max chunk size.

+1 for this patch. This patch fixes large podman images (tested things over ~100MB) which previously were erroring out during chunk uploads with Foreman/Katello.

[MSP-Greg]

I just rebased and added another test that I only tested locally on Ubuntu. Locally, it fails on master, passes with this PR.

Of course, just to make things fun, it fails on macOS & Windows (note to self - test all OS's locally). I'll either remove it and save for later, or fix it. Either way, I'll merge this later today...

[ianballou]

Great to see this merge, we'll test it out again as soon as it makes it into a gem.