php-wasm : Symlink infinite loop

[mho22]

I have a directory public used as documentroot. This directory has 3 files :

- public
    - directory
        - subfile.php -> echo "DIRECTORY";
    - store ~symlink~
    - file.php -> echo "PUBLIC";
- store
    - storefile.php -> echo "STORE";

The symlink returns the store directory at the root level, same level as public

The symlink has been created with a php script symlink.php :

<?php

symlink( __DIR__.'/store', __DIR__.'/public/store' );

import { NodePHP } from "@php-wasm/node";


const php = await NodePHP.load( "8.2", { requestHandler : { documentRoot : 'public', absoluteUrl : 'localhost:2222' } } );

php.mount( process.cwd(), '/php-wasm' );

php.chdir( `/php-wasm` );

When trying :

http://localhost:2222/file.php -> "PUBLIC"
http://localhost:2222/directory/subfile.php -> "DIRECTORY"
http://localhost:2222/store/storefile.php -> 404 NOT FOUND

I found out the method called here to make this work is lookupPath().

The problem is : that same lookupPath() method calls FS.readlink(current_path). And that same readlinkmethod calls lookupPath() inside of it. If the current_path doesnt change in between them, it creates a sort of inifinite loop.

And that is what is happening here. In fact, the return value of the readlink method is always the same result. In my example it is php-wasm/public/store.

And if I separate elements :

 PATH_FS.resolve(FS.getPath(link.parent), link.node_ops.readlink(link)); --> `php-wasm/public/store`
 
 FS.getPath(link.parent) --> `php-wasm/public`
 
link.node_ops.readlink(link) --> `store`

To resolve this, I modified the return value of the readlink method :

// Before 

return PATH_FS.resolve(FS.getPath(link.parent), link.node_ops.readlink(link));

// After

var parent = PATH_FS.resolve(FS.getPath(link.parent), linkpath) === path ? link.parent.parent : link.parent;

return PATH_FS.resolve(FS.getPath(parent), link.node_ops.readlink(link));

Waiting for your confirmation before creating a PR for this.

[adamziel]

@mho22 this makes sense, good find! This seems like an Emscripten issue, I think it's this line:

https://github.com/emscripten-core/emscripten/blob/b7fdff5c53410115fef88a78be84cdc8671fbb37/src/library_fs.js#L907

If you'd open a PR for that project, it would fix the problem for everyone and we wouldn't have to patch the Emscripten output in here. If it's not reviewed and released reasonably quickly, let's explore an in-Playground patch after all.

[mho22]

@adamziel new Pull Request created.

[mho22]

@adamziel Emscripten Pull Request merged ! 🎉. I assume this will be considered from now on, correct? Should I suggest a Pull Request with up-to-date builds ?

[adamziel]

@mho22 We'll need to wait for the next Emscripten release and then update the Emscripten version used by the base Dockerfile. From there, rebuilding will do the trick. Thank you so much for your amazing work and going as far as fixing the upstream project!

[brandonpayton]

@mho22 I read through the Emscripten PR. Nice work!

[mho22]

@adamziel @brandonpayton Thanks, everyone! I'll make sure to keep an eye out for the next Emscripten release.

[mho22]

@adamziel @brandonpayton Emscripten 3.1.60 is available. Do you think I should update current version 3.1.43 to version 3.1.60 ?

[brandonpayton]

👋 Hi @mho22! It looks like there might be a reason we are still back on the 3.1.43 version.

The Emscripten v3.1.44 changes include:

The asm property of the Module object (which held the raw exports of the wasm module) has been removed. Internally, this is now accessed via the wasmExports global. If necessary, it is possible to export wasmExports on the Module object using -sEXPORTED_RUNTIME_METHODS=wasmExports. (#19816)

And it looks like we currently reference the asm property for error reporting.

Unless @adamziel knows other reasons we haven't upgraded, I imagine it would be good to upgrade if the dependency on the asm property is adjusted so error logging improvements still work. (We might also have other uses of that property, but that was the one I found.)

[mho22]

Hi @adamziel ! What do you think about @brandonpayton said about this ?