Improve support for concurrent requests
mxschmitt opened this issue · 6 comments
Currently the behaviour is, that for that the files get messed up and some users will get files of other users.
From my understanding we have to change in the end the current working directory of the main process, because Playwright will use fs.writeFile if no absolute path is specified.
Another approach which should be combined in the end in my mind is to outsource the API endpoint in an own Docker based microservice. They will communicate over RabbitMQ to share there messages and upload the assets into some blob storage like S3.
I was looking at vm2 docs, and it seems we can mock the fs module if we use NodeVM instead of VM. I don't think the isolation trade offs justify using NodeVM instead of VM.
Also, I found that it's possible to break the current implementation by passing an absolute path. To prevent that (and solve this) I think we can explore overriding path: * in the code. It should be possible to do this with a good regex. What do you think?
Currently we are passing the playwright object from externally into the VM. I also tried out the approach with using NodeVM with a require whitelist but it worked only partly. Execution seems to be successfully but returning from it or something like that was not working.
If we use NodeVM we can also emulate fs, that would be the best.
Regarding the path escape issue, if we emulate fs we would not care about it right?
Regarding the path escape issue, if we emulate fs we would not care about it right?
Yes mocking fs will fix it. We could explore this as a replacement for fs inside the NodeVM: https://github.com/streamich/memfs
Some status update regarding that:
- Use patch-package to modify Playwright for our needs -> complex and not elegant
- Patch
fs.writeFile,fs.open,fs.writeandfs.close-> we dont have the context to the request and its process wide - Change
process.cwd()viaprocess.chdir()during the runtime (once a request comes in) -> will probably be process wide -> issue not solved