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

OpenSwoole: Streamed response adds invalid "Transfer-Encoding: chunked" header #568

Closed
Radiergummi opened this issue Aug 29, 2022 · 6 comments
Assignees
Labels

Comments

@Radiergummi
Copy link

Radiergummi commented Aug 29, 2022

  • Octane Version: 1.3.0 (current, see repro)
  • Laravel Version: 9.26.1 (current, see repro)
  • PHP Version: 8.1.2
  • Server & Version: Swoole / RoadRunner 4.10.0
  • Database Driver & Version: -

Description:

When using Octane with the swoole integration, streaming downloads from the filesystem don't work with certain software because an illegal header combination is added to the response; as per RFC 2616, Section 4.4, the Content-Length and Transfer-Encoding headers may not be used together:

"Messages MUST NOT include both a Content-Length header field and a non-identity transfer-coding. If the message does include a non-identity transfer-coding, the Content-Length MUST be ignored."

The following code sample demonstrates the issue:

Route::get('/download', static function (FilesystemManager $filesystem) {
    return $filesystem->disk('local')->download(
        'public/test.jpg',
        'test-download.jpg',
        ['Content-Type' => 'image/jpeg']
    );
});

This yields the following response:

HTTP/1.1 200 OK
Cache-Control: no-cache, private
Connection: keep-alive
Content-Disposition: attachment; filename=test-download.jpg
Content-Length: 515879
Content-Type: image/jpeg
Date: Mon, 29 Aug 2022 15:04:05 GMT
Server: OpenSwoole 4.10.0
Set-Cookie: XSRF-TOKEN=(token here); expires=Mon, 29-Aug-2022 17:04:05 GMT; path=/; samesite=lax
Set-Cookie: laravel_session=(session key here); expires=Mon, 29-Aug-2022 17:04:05 GMT; path=/; httponly; samesite=lax
Transfer-Encoding: chunked

(binary content here)

As you can see, there's a Content-Length, and a Transfer-Encoding: chunked header in the response. This causes nginx to throw up, for example (log entry from production):

f1nih 2022/08/29 09:17:39 [error] 32#32: *78 upstream sent "Content-Length" and "Transfer-Encoding"
headers at the same time while reading response header from upstream

The issue does not occur with the RoadRunner server enabled. Also, this might be related to #109.

Steps To Reproduce:

See the reproduction repository:
matchory/laravel-octane-bug-report-transfer-encoding

  1. Install dependencies using composer install
  2. Start the server using php artisan octane:start
  3. Perform a GET request on the /download endpoint, for example using cURL:
    curl -IX GET http://localhost:8000/download
  4. Observe the response headers
@Radiergummi
Copy link
Author

As a temporary fix, I removed the Content-Length header by setting it to null:

Route::get('/download', static function (FilesystemManager $filesystem) {
    return $filesystem->disk('local')->download(
        'public/test.jpg',
        'test-download.jpg',
-        ['Content-Type' => 'image/jpeg']
+        ['Content-Type' => 'image/jpeg', 'Content-Length' => null]
    );
});

This precludes Laravel from setting its own Content-Length header, so the response won't contain any, and nginx doesn't complain. This makes it impossible to have download progress on client browsers, however, so this is merely a temporary workaround.

@nunomaduro
Copy link
Member

Taking a look at this next week.

@nunomaduro
Copy link
Member

Sorry, but the fact that this issue only exists on Swoole, and not on RoadRunner / Regular Nginx, tells me that this is an issue with Swoole itself. Please reach out to the authors library here: https://github.com/swoole/swoole-src/issues.

@Radiergummi
Copy link
Author

@nunomaduro understandable. Might you have any pointers for me which OpenSwoole APIs Laravel uses? I'm a bit into the code, but not that deep.

@ignacioalles
Copy link

The same situation occurs with swoole (tested with 4.8.10 and 5.0.0), octane 1.3 and it also occurs with not streamed responses with explicit Content-Length like

return response('Hello', 200, [ 'Content-Length' => 5  ]);

@ignacioalles
Copy link

It seems to be a bad behaviour of Swoole\Http\Response::write(string $content) method, since end(string $content) method works as expected.

The following test sample using write() method to send content:

$http->on('Request', function (Request $request, Response $response) {
    $response->header('Content-Length', 5);
    $response->write('Hello');
    $response->end();
});

produces:

HTTP/1.1 200 OK
Content-Length: 5
Server: swoole-http-server
Connection: keep-alive
Content-Type: text/html
Date: Wed, 21 Sep 2022 15:47:19 GMT
Transfer-Encoding: chunked

Hello

while sending content in end() like:

$http->on('Request', function (Request $request, Response $response) {
    $response->header('Content-Length', 5);
    $response->end('Hello');
});

does not include Transfer-Encoding header:

HTTP/1.1 200 OK
Content-Length: 5
Server: swoole-http-server
Connection: keep-alive
Content-Type: text/html
Date: Wed, 21 Sep 2022 15:49:46 GMT

Hello

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants