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

Asyncify: Test all network functions and all ways to trigger a function call #273

Closed
adamziel opened this issue May 9, 2023 · 2 comments
Labels
[Aspect] Asyncify [Feature] PHP.wasm [Priority] Low [Type] Bug An existing feature does not function as intended [Type] Enhancement New feature or request

Comments

@adamziel
Copy link
Collaborator

adamziel commented May 9, 2023

Description

As explained in #251 and #253, all PHP code paths leading to an asynchronous network call must be listed in php-wasm/compile/Dockerfile. In the longer term, we'll move from Asyncify to JSPI API that has a much simpler configuration, but for now we'll need to maintain the ASYNCIFY_ONLY list.

Finding them manually would be tedious or impossible, so #253 introduced a test suite that detects and auto-adds any missing code paths. That test suite should cover the following two concepts as exhaustively as possible:

Network functions

PHP functions directly responsible for triggering a network call like file_get_contents, fsockopen, mysql_connect should be listed here:

const topOfTheStack: Array<string> = [
// http:// stream handler
`file_get_contents(${js['httpUrl']});`,
`$fp = fopen(${js['httpUrl']}, "r");
fread($fp, 1024);
fclose($fp);`,
// `getimgsize(${js['httpUrl']});`,
// Network functions from https://www.php.net/manual/en/book.network.php
`$fp = fsockopen(${js['host']}, ${js['port']});
fwrite($fp, "GET / HTTP/1.1\\r\\n\\r\\n");
fread($fp, 10);
fclose($fp);`,
`gethostbyname(${js['httpUrl']});`,
// @TODO:
// https:// stream handler
// MySQL functions from https://www.php.net/manual/en/book.mysql.php
// PDO functions from https://www.php.net/manual/en/book.pdo.php
// Sockets functions from https://www.php.net/manual/en/book.sockets.php
];

A bunch of them already is, but there's also a TODO comment listing the missing ones

Function calls

Different ways of calling the network functions like a direct function call, method call, reflections API are covered by specific test cases here:

test('Direct call', () => assertNoCrash(` ${networkCall}`));
describe('Function calls', () => {
test('Simple call', () =>
assertNoCrash(`function top() { ${networkCall} } top();`));
test('Via call_user_func', () =>
assertNoCrash(
`function top() { ${networkCall} } call_user_func('top'); `
));
test('Via call_user_func_array', () =>
assertNoCrash(
`function top() { ${networkCall} } call_user_func_array('top', array());`
));
});
describe('Class method calls', () => {

The initial list is nice, but we should also cover destructors, shutdown functions, serializable interface, and anything else you can think of.

Rebuilding PHP

After adding a test case, run

node --stack-trace-limit=100 ./node_modules/.bin/nx test php-wasm-node --test-name-pattern='asyncify'

If all the tests are green, good. If they're not, you may auto-fix the Dockerfile and rebuild PHP using this command:

npm run fix-asyncify

This takes some time so you may want to add all your test cases first and then run fix-asyncify just once at the end. Once it completes, all your new tests should pass.

@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Type] Enhancement New feature or request [Aspect] Asyncify labels May 9, 2023
@adamziel adamziel changed the title Add more Asyncify test cases Asyncify: Add more test cases May 9, 2023
@adamziel adamziel changed the title Asyncify: Add more test cases Asyncify: Test all network functions and all ways to trigger a function call May 9, 2023
@adamziel
Copy link
Collaborator Author

As identified in #387, the following code crashes PHP versions 5.6, 7.2, 8.2, and correctly throws an exception on all other versions:

<?php
class Top {
	function __clone() {
		file_get_contents("http://127.0.0.1");
	}
}
$x = new Top();
clone $x;

@adamziel
Copy link
Collaborator Author

adamziel commented Oct 9, 2024

Asyncify is now deprecated in favor of JSPI. Let's drop this stream of work and only do light maintenance of the Asyncify builds. Chrome and Firefox users are now on JSPI builds. Over time, Safari, other browsers, and the LTS Node.js release will start offering JSPI support and we'll be able to remove the Asyncify builds entirely.

@adamziel adamziel closed this as not planned Won't fix, can't repro, duplicate, stale Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Aspect] Asyncify [Feature] PHP.wasm [Priority] Low [Type] Bug An existing feature does not function as intended [Type] Enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

1 participant