PHP.wasm: fix stack overflow in wasm_set_request_body #993
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before this PR, requests with large body trigger a stack overflow error. For example, issue #816 describes how clicking a "Use template" button calls wasm_set_request_body with a body string that's 153KB large which is too much for the current WASM build. This PR fixes the issue by allocating the request body on the heap, not on the stack. ## Implementation details `BasePHP.setRequestBody` converts the body string into bytes using the `stringToUTF8` function provided by Emscripten. This is just as it was before this PR. This method isn't perfect and will break for some inputs. The `setRequestBody` method should just accept a `UInt8Array` instead of a string, but that's outside of scope of this PR. ## Testing instructions Confirm the CI checks are green. This PR ships with a series of checks to confirm the crash is fixed. For manual testing, navigate to https://beemovie.fandom.com/wiki/Bee_Movie/Transcript and Select All, then Copy the selection. Then go to http://localhost:5400/website-server/?php=8.3&wp=6.4&storage=none&url=/wp-admin/post-new.php and paste the content of the clipboard directly into the editor. The javascript will take a moment to digest the pasted content into blocks. The editor should end up creating very many blocks. Once thats done, click publish, and the post should save without crashing.
adamziel
added
[Type] Bug
|
The failing test is flaky – it timed out. All the important checks worked. Let's merge and address the flaky tests separately. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this PR, requests with large body trigger a stack overflow error. For example, issue #816 describes how clicking a "Use template" button calls wasm_set_request_body with a body string that's 153KB large which is too much for the current WASM build.
This PR fixes the issue by allocating the request body on the heap, not on the stack.
Implementation details
BasePHP.setRequestBodyallocates enough heap memory to store the body string. Then, it converts the body string into bytes and writes it into heap. Once the request is finished,BasePHP.run()frees that memory.The conversion to bytes is done via the
stringToUTF8function provided by Emscripten. This is just as it was before this PR. This method isn't perfect and will break for some inputs. ThesetRequestBodymethod should just accept aUInt8Arrayinstead of a string, but that's outside of scope of this PR.Testing instructions
Confirm the CI checks are green. This PR ships with a series of checks to confirm the crash is fixed.
For manual testing, navigate to https://beemovie.fandom.com/wiki/Bee_Movie/Transcript and Select All, then Copy the selection.
Then go to http://localhost:5400/website-server/?php=8.3&wp=6.4&storage=none&url=/wp-admin/post-new.php and paste the content of the clipboard directly into the editor. The javascript will take a moment to digest the pasted content into blocks. The editor should end up creating very many blocks. Once thats done, click publish, and the post should save without crashing.
Supersedes #870