Repository navigation
Fix corrupt zip download from Site Editor export - #2531
Conversation
brandonpayton
added
[Type] Bug
|
If we stop decoding and encoding stdout as part of the streamed PHP response (as in 04c8eba), the download corruption is no longer an issue. It seems like PHPResponse or the streamed response should just be relaying the bytes and the headers, and the consumer of the response can decide how to handle the bytes. |
|
@adamziel, since all tests pass and this looks good to me, I'll merge it, but please @-mention me if we need to take another approach. Thank you! |
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I started to look at this PR yesterday afternoon, but then had to go. However, I thought there was a one-liner to do this, or at least a simple way, and now I found it:
const bytes = await new Response(stream).bytes();It might be worth considering for the simplicity of that code.
There was a problem hiding this comment.
const bytes = await new Response(stream).bytes();
@JanJakes Nice and thank you, I hadn't considered that!
Motivation for the change, related issues
The purpose of this PR is to fix the corrupt zip download from the Site Editor "Export" feature.
Fixes #2528
Implementation details
This PR adds a
stdoutBytesproperty to StreamedPHPResponse and updatesPHPResponse.fromStreamedResponse()to use that.Prior to this change,
fromStreamedResponse()used thestdoutTextproperty which took a byte stream that was decoded on the fly to text and then re-encoded by the accessor to bytes. The bytes of a zip file going through that transform were being corrupted. Without that transform, the zip file bytes are not corrupted.Testing Instructions (or ideally a Blueprint)
npx nx unbuilt-asyncify playground-cli -- server