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

Update: Simplify base-php sapi_handle_request call #387

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 87 additions & 1 deletion packages/php-wasm/node/src/test/php.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { getPHPLoaderModule, NodePHP } from '..';
import { loadPHPRuntime, SupportedPHPVersions } from '@php-wasm/universal';
import {
loadPHPRuntime,
SupportedPHPVersions,
__private__dont__use,
} from '@php-wasm/universal';
import { existsSync, rmSync, readFileSync } from 'fs';

const testDirPath = '/__test987654321';
Expand Down Expand Up @@ -554,3 +558,85 @@ bar1
});
});
});

// @TODO Prevent crash on PHP versions 5.6, 7.2, 8.2
adamziel marked this conversation as resolved.
Show resolved Hide resolved
describe.each(['7.0', '7.1', '7.3', '7.4', '8.0', '8.1'])(
'PHP %s – process crash',
(phpVersion) => {
let php: NodePHP;
beforeEach(async () => {
php = await NodePHP.load(phpVersion as any);
php.setPhpIniEntry('allow_url_fopen', '1');
vi.restoreAllMocks();
});

it('Does not crash due to an unhandled Asyncify error ', async () => {
let caughtError;
try {
/**
* PHP is intentionally built without network support for __clone()
* because it's an extremely unlikely place for any network activity
* and not supporting it allows us to test the error handling here.
*
* `clone $x` will throw an asynchronous error out when attempting
* to do a network call ("unreachable" WASM instruction executed).
* This test should gracefully catch and handle that error.
*
* A failure to do so will crash the entire process
*/
await php.run({
code: `<?php
class Top {
function __clone() {
file_get_contents("http://127.0.0.1");
}
}
$x = new Top();
clone $x;
`,
});
} catch (error: unknown) {
caughtError = error;
if (error instanceof Error) {
expect(error.message).toMatch(
/Aborted|Program terminated with exit\(1\)|/
);
}
}
if (!caughtError) {
expect.fail('php.run should have thrown an error');
}
});

it('Does not crash due to an unhandled non promise error ', async () => {
let caughtError;
try {
const spy = vi.spyOn(php[__private__dont__use], 'ccall');
expect(spy.getMockName()).toEqual('ccall');
spy.mockImplementation((c_func) => {
if (c_func === 'wasm_sapi_handle_request') {
throw new Error('test');
}
});

await php.run({
code: `<?php
function top() {
file_get_contents("http://127.0.0.1");
}
top();
`,
});
} catch (error: unknown) {
caughtError = error;
if (error instanceof Error) {
expect(error.message).toMatch('test');
expect(error.stack).toContain('#handleRequest');
}
}
if (!caughtError) {
expect.fail('php.run should have thrown an error');
}
});
}
);
29 changes: 10 additions & 19 deletions packages/php-wasm/universal/src/lib/base-php.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
let errorListener: any;
try {
// eslint-disable-next-line no-async-promise-executor
exitCode = await new Promise<number>(async (resolve, reject) => {
exitCode = await new Promise<number>((resolve, reject) => {
errorListener = (e: ErrorEvent) => {
const rethrown = new Error('Rethrown');
rethrown.cause = e.error;
Expand All @@ -396,25 +396,16 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
'error',
errorListener
);

try {
resolve(
/**
* This is awkward, but Asyncify makes wasm_sapi_handle_request return
* Promise<Promise<number>>.
*
* @TODO: Determine whether this is a bug in emscripten or in our code.
*/
await await this[__private__dont__use].ccall(
'wasm_sapi_handle_request',
NUMBER,
[],
[]
)
);
} catch (e) {
reject(e);
const response = this[__private__dont__use].ccall(
'wasm_sapi_handle_request',
NUMBER,
[],
[]
);
if (response instanceof Promise) {
return response.then(resolve, reject);
}
return resolve(response);
});
} catch (e) {
/**
Expand Down