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.

Sign up for GitHub

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

Jump to bottom

$_FILES[...]['error'] should be int and not string #914

Closed
TobiasBg opened this issue Jan 7, 2024 · 1 comment · Fixed by #1018
Closed

$_FILES[...]['error'] should be int and not string #914

TobiasBg opened this issue Jan 7, 2024 · 1 comment · Fixed by #1018
Labels
[Feature] PHP.wasm [Type] Bug An existing feature does not function as intended

Comments

@TobiasBg
Copy link

TobiasBg commented Jan 7, 2024

In my plugin, I'm using UPLOAD_ERR_OK !== $_FILES['form-field-name']['error'] to check for issues with file uploads (from an <input type="file" name="form-field-name" /> in this example).

The UPLOAD_ERR_OK constant in PHP normally is an int, see https://3v4l.org/WjW2j .

However, in WordPress Playground, $_FILES['form-field-name']['error'] returns a string.

Maybe something in

wordpress-playground/packages/php-wasm/compile/php/php_wasm.c

Line 1184 in 57ef882

// Set $_FILES['key']['error']
needs to be adjusted?
@adamziel
Copy link
Collaborator

adamziel commented Jan 9, 2024

Good find @TobiasBg! It sounds like php_register_variable_ex would be a better choice as it accepts a zval instead of a char. This is what PHP does (through a few layers of indirection):

https://github.com/php/php-src/blob/d7d0d19d3279c040b852b7b8c3c38083cd29e4fd/main/rfc1867.c#L1232C5-L1232C41

@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Feature] PHP.wasm labels Jan 9, 2024
@adamziel adamziel mentioned this issue Feb 3, 2024
Open
adamziel added a commit that referenced this issue Feb 8, 2024
@adamziel
PHP: Pass request body as Uint8Array
720546f 
Removes the custom file upload handler and rely on PHP body parsing
to populate the $_FILES array. Instead of encoding the body bytes as
a string, parsing that string, and re-encoding it as bytes, we keep
the body in a binary form and pass it directly to PHP HEAP memory.

Closes #997
Closes #1006
Closes #914

 ## Testing instructions

Confirm the CI checks pass (it will take a few iterations to get them right I'm sure :D)
@adamziel adamziel mentioned this issue Feb 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Feature] PHP.wasm [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

PHP: Pass request body as UInt8Array WordPress/wordpress-playground
2 participants
@TobiasBg @adamziel