Tests: client_body_early_read directive. by jimf5 · Pull Request #103 · nginx/nginx-tests

jimf5 · 2026-08-15T07:02:48Z

Proposed changes

Added tests for the client_body_early_read directive introduced in PR: nginx/nginx#1641

Checklist

Before creating a PR, run through this checklist and mark each as complete:

I have read the contributing guidelines.
I have signed the F5 Contributor License Agreement (CLA).
If applicable, I have added tests that prove my fix is effective or that my feature works.
If applicable, I have checked that any relevant tests pass after adding my changes.
I have updated any relevant documentation (README.md and/or CHANGELOG.md).

pluknet · 2026-08-19T11:35:09Z

+plan(skip_all => 'no client_body_early_read support')
+    unless $t->has_version('1.31.4');


spaces to tabs

or remove "support" to unwrap into a single line, or use $t->try_run()

also, let's drop a trailing dot in log summary

Thanks, fixed.

pluknet

approved with comments

route443 · 2026-08-27T22:53:47Z

+        server_name  localhost;
+
+        client_max_body_size 256;
+        client_body_buffer_size 4k;


It seems that none of the checks depend on this directive value, the only checks regarding the buffer/file go to server 8083, which has its own client_body_buffer_size 16. If I remove it, tests are passed.

Yes, this line is not needed. Good catch.

route443 · 2026-08-27T23:06:31Z

+        listen       127.0.0.1:8081;
+        server_name  localhost;
+
+        client_body_early_read $arg_on;


Here we check the complex val only, which is good, however, the directive itself employs a more extensive logic:

0 if we want to disable it (falsy literal)

multiple arguments are possible => the rule acts as OR

Added explicit tests for 0 and non-0 cases

route443 · 2026-08-27T23:12:46Z

+	'body at exact limit');
+like(http_post_body('/', '0123456789' x 2, 8082), qr/ 413 /,
+	'server limit applies not location');
+like(http_post_body('/', '0123456789' x 100, 8083), qr/ 204 /,


what exactly are we trying to test here? If we remove client_max_body_size 0;, then this test will pass...

Yes, the test name is misleading. Renamed. The goal of this test is to confirm that intensive memory allocation does not impact system functionality or stability.

route443 · 2026-08-27T23:20:54Z

+        }
+    }
+
+    server {


I don't think this server is needed at all, since 8002 is perfectly sufficient. The only difference is the client_max_body_size 100 setting within the location block, but that gets ignored in early read mode anyway. In other words, the chunked body exceeds limit and chunked body within limit cases can be moved to 8002

Yes, you are right.

route443 · 2026-08-27T23:27:52Z

+
+like(http_post_body('/', '0123456789', 8080), qr/body:0123456789/,
+	'body via return');
+unlike(http_post_body('/', '0123456789', 8081), qr/body:0123456789/,


I'd try to avoid unlike tests, since they succeed on just about anything. I would do:

like(http_post_body('/', '0123456789', 8081), qr/body:$/, 'predicate false');

Makes sense; fixed.

route443 · 2026-08-27T23:28:46Z

+unlike(http_post_body('/', '0123456789', 8081), qr/body:0123456789/,
+	'no early read - body not available');
+like(http_post_body('/?on=1', '0123456789', 8081), qr/body:0123456789/,
+	'ealy read - body available');


ealy -> early

route443 · 2026-08-27T23:38:20Z

+	'no early read - body not available');
+like(http_post_body('/?on=1', '0123456789', 8081), qr/body:0123456789/,
+	'ealy read - body available');
+like(http_post_body('/proxy', '0123456789', 8080),


during feature discussion, we mentioned this directive isn't needed for proxy_pass, meaning you'll get the body in the content phase anyway. So, even if you remove the directive, this test (and friends) will still pass...

This test is intended to ensure that no regressions have been introduced.

route443 · 2026-08-28T01:44:42Z

+	qr/X-Body: 0123456789\x0d?$/ms, 'body via proxy');
+like(http_get('/'), qr/ 200 /, 'no body');
+like(http_post_body('/', '', 8080), qr/body:$/, 'empty body');
+like(http_post_body('/', '0123456789', 8080), qr/ 200 /,


This is the same test as body via return, but the regex is different, but body via return is only possible with 200 ok => can be removed

Makes sense, removed

route443 · 2026-08-28T01:50:59Z

+like(http_post_body('/', '', 8080), qr/body:$/, 'empty body');
+like(http_post_body('/', '0123456789', 8080), qr/ 200 /,
+	'body within limit');
+like(http_post_body('/', 'x' x 300, 8080), qr/ 413 /,


Duplicate of server limit applies not location. The location block L:51 does not have its own limit => the location value matches the server value.

Updated tests to show directive inheritance.

route443 · 2026-08-28T01:55:07Z

+	'body within limit');
+like(http_post_body('/', 'x' x 300, 8080), qr/ 413 /,
+	'body exceeds limit');
+like(http_post_body('/', 'x' x 256, 8080), qr/ 200 /,


I'd replace qr/ 200 / to qr/body:x{256}/.

route443 · 2026-08-28T02:07:54Z

+	'early read body reaches location');
+like(http_post_body('/rewrite', 'HELLO', 8080), qr/loc1:HELLO/,
+	'early read body survives rewrite');
+like(http_post_body('/echo', 'TESTBODY', 8080), qr/echo:TESTBODY/,


looks like it is also redundant, since server 8085 inherits client_body_early_read 1 from http block and proxy_pass passes the body upstream in any case.

Agree. No, this test isn't needed.

route443 · 2026-08-28T02:16:00Z

+like($r, qr/ 413 /, 'expect continue too large');
+unlike($r, qr/100 Continue/, 'expect continue too large - no 100');


the same remark as for body exceeds limit, It looks like you need to send the request to a location with a limit higher than the server limit.

No, it's intended to verify that no 100 status in the 413 response.

route443 · 2026-08-28T02:21:19Z

+like($r, qr/ 413 /, 'expect continue too large');
+unlike($r, qr/100 Continue/, 'expect continue too large - no 100');
+
+like(http_post_body('/loc1', 'HELLO', 8080), qr/loc1:HELLO/,


I don't quite see the fundamental difference compared to body via return tc

Yes, me too. Removed.

jimf5

@route443 Thanks for the review

jimf5 · 2026-08-30T18:15:53Z

+        server_name  localhost;
+
+        client_max_body_size 256;
+        client_body_buffer_size 4k;


Yes, this line is not needed. Good catch.

jimf5 · 2026-08-30T18:20:13Z

+        listen       127.0.0.1:8081;
+        server_name  localhost;
+
+        client_body_early_read $arg_on;


Added explicit tests for 0 and non-0 cases

jimf5 · 2026-08-30T18:22:49Z

+        }
+    }
+
+    server {


Yes, you are right.

jimf5 · 2026-08-30T18:23:07Z

+unlike(http_post_body('/', '0123456789', 8081), qr/body:0123456789/,
+	'no early read - body not available');
+like(http_post_body('/?on=1', '0123456789', 8081), qr/body:0123456789/,
+	'ealy read - body available');


jimf5 · 2026-08-30T18:28:14Z

+	'no early read - body not available');
+like(http_post_body('/?on=1', '0123456789', 8081), qr/body:0123456789/,
+	'ealy read - body available');
+like(http_post_body('/proxy', '0123456789', 8080),


This test is intended to ensure that no regressions have been introduced.

jimf5 · 2026-08-30T19:04:44Z

+	'body within limit');
+like(http_post_body('/', 'x' x 300, 8080), qr/ 413 /,
+	'body exceeds limit');
+like(http_post_body('/', 'x' x 256, 8080), qr/ 200 /,


jimf5 · 2026-08-30T19:08:32Z

+like($r, qr/ 413 /, 'expect continue too large');
+unlike($r, qr/100 Continue/, 'expect continue too large - no 100');
+
+like(http_post_body('/loc1', 'HELLO', 8080), qr/loc1:HELLO/,


Yes, me too. Removed.

jimf5 · 2026-08-30T19:18:51Z

+	'early read body reaches location');
+like(http_post_body('/rewrite', 'HELLO', 8080), qr/loc1:HELLO/,
+	'early read body survives rewrite');
+like(http_post_body('/echo', 'TESTBODY', 8080), qr/echo:TESTBODY/,


Agree. No, this test isn't needed.

jimf5 · 2026-08-30T19:37:33Z

+	'body at exact limit');
+like(http_post_body('/', '0123456789' x 2, 8082), qr/ 413 /,
+	'server limit applies not location');
+like(http_post_body('/', '0123456789' x 100, 8083), qr/ 204 /,


Yes, the test name is misleading. Renamed. The goal of this test is to confirm that intensive memory allocation does not impact system functionality or stability.

jimf5 · 2026-08-30T19:52:55Z

+like($r, qr/ 413 /, 'expect continue too large');
+unlike($r, qr/100 Continue/, 'expect continue too large - no 100');


No, it's intended to verify that no 100 status in the 413 response.

jimf5 requested review from arut, pluknet and route443 August 15, 2026 07:02

jimf5 self-assigned this Aug 15, 2026

jimf5 added the tests label Aug 15, 2026

jimf5 added this to NGINX OSS Unified Workspace Aug 15, 2026

github-project-automation Bot moved this to New in NGINX OSS Unified Workspace Aug 15, 2026

jimf5 force-pushed the dev/client-body-preread branch from 062294f to b6bb50b Compare August 15, 2026 07:55

sindhushiv moved this from New to In Review in NGINX OSS Unified Workspace Aug 17, 2026

jimf5 force-pushed the dev/client-body-preread branch from b6bb50b to 4d25224 Compare August 17, 2026 18:56

pluknet reviewed Aug 19, 2026

View reviewed changes

pluknet approved these changes Aug 19, 2026

View reviewed changes

jimf5 force-pushed the dev/client-body-preread branch 2 times, most recently from 4daf606 to 18f9433 Compare August 20, 2026 08:45

jimf5 force-pushed the dev/client-body-preread branch from 18f9433 to 33c136d Compare August 27, 2026 19:48

route443 requested changes Aug 27, 2026

View reviewed changes

github-project-automation Bot moved this from In Review to In Progress in NGINX OSS Unified Workspace Aug 27, 2026

route443 requested changes Aug 28, 2026

View reviewed changes

Tests: client_body_early_read directive

b6c8564

jimf5 force-pushed the dev/client-body-preread branch from f250d85 to b6c8564 Compare August 30, 2026 19:41

jimf5 commented Aug 30, 2026

View reviewed changes

		plan(skip_all => 'no client_body_early_read support')
		unless $t->has_version('1.31.4');

		like($r, qr/ 413 /, 'expect continue too large');
		unlike($r, qr/100 Continue/, 'expect continue too large - no 100');

Uh oh!

Conversation

jimf5 commented Aug 15, 2026

Proposed changes

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pluknet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimf5 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!