fix: don't flush if the request body hasn't been read entirely

[dunglas]

Follows #121. Mitigate golang/go#15527.

[jfcoz]

Hello
I’ve started a POC with wordpress + FrankenPHP : https://github.com/jfcoz/wptest
Uploads are uploaded to a bucket via some wordpress plugins that you can enable :

• humamade/s3-upload plugin
• OR
• ilab-media-cloud plugin

You can run it with docker-compose, or in Kubernetes.

During a media upload, I get the error error while reading the request body:

• only if it runs behind an nginx ingress controller in Kubernetes
• AND
• only with the ilab-media-cloud plugin

Your fix already mitigate the crash I get before, but I’m curious to understand the root cause.

[withinboredom]

What a strange bug... 🤔

[smortexa]

@dunglas This issue still exists. During an upload (successful) with Octane, the error while reading the request body log prints.

[dunglas]

Thanks for the feedback! Would you been able to provide a reproducer?

[smortexa]

ASAP. I found that this happens for non-upload requests eventually.

[smortexa]

I tried to reproduce but I faced this #566 (comment) again I couldn't run FrankenPHP with my Dockerfile in my M1 machine. It seems this issue does not exist in Laravel Sail or non-containerized environment.

[smortexa]

@dunglas Here is the reproduction repo of this issue:
https://github.com/smortexa/frankenphp-reproduce-566#steps-to-reproduce-error-while-reading-the-request-body

[jskorlol]

I too am having this problem. It seems that request_body is limited to 2MB in size.

There is no problem uploading files less than 2MB, but files larger than 2MB will result in an ERROR error while reading the request body error.

http://:9000 {
	request_body {
		max_size 10MB
	}
}

What I don't know is that it doesn't seem to work even though I applied that option.

[withinboredom]

Hmm, I think it makes sense that we are seeing this error, looking at the code.

1. It looks like the issue this error condition was designed to solve was actually fixed in go (after over six years of being open!), fairly recently. So this code can probably be deleted and probably doesn't work the same as it once did, now that concurrently reading/writing is possible.
2. If PHP doesn't request enough bytes (2mb chunks sounds pretty reasonable), it will give this error.

This looks like it might be an easy fix since the underlying go issue might be fixed.

[withinboredom]

@smortexa I've opened a PR to your reproducer with some dockerfile suggestions. Thank you for making it, as it made verifying the fix much easier.

[withinboredom]

If you'd want to give this image a go: dunglas/frankenphp-dev:latest-8.3-bookworm to validate it is fixed, that would be epic.