Allow "wasm-unsafe-eval" in CSP
If a page has a Content Security Policy header and the script-src (or default-src) directive does not contain neither wasm-unsafe-eval nor unsafe-eval loading and executing WebAssembly is blocked in the page (although it is still possible to load and execute WebAssembly in a worker thread).
Although the Nextcloud classes to manage the CSP already supported allowing unsafe-eval this affects not only WebAssembly, but also the eval operation in JavaScript.
To make possible to allow WebAssembly execution without allowing JavaScript eval this commit adds support for allowing wasm-unsafe-eval (which is supported since some versions already by browsers).
Nevertheless, please keep in mind that running heavy WebAssembly code in the main thread can lead to unresponsiveness in the rest of the UI; it might be better to run WebAssembly code in a worker thread and do it in the main thread only for light (so it does not block the rest of the code) real time (which could lead to issues if run in a worker thread due to the synchronization of the data between the threads) calculations.
As per https://help.nextcloud.com/t/changes-to-documentation-handling/161436 this needs to be documented
I do not see where null and false mean something different, why is the property nullable? Could it be a bool initialized to false or is it ever useful to know if it was set to false or never touched?
To be honest I do not really know, I also wondered the same, but I just followed the existing code :shrug:
To be honest I do not really know, I also wondered the same, but I just followed the existing code shrug
Don’t do that, fix the existing code!
But at least for this PR use bool instead of ?bool in all places, it’s clearer.
To be honest I do not really know, I also wondered the same, but I just followed the existing code shrug
Don’t do that, fix the existing code!
Well, of course, that is what I usually try to do, but not in this case ;-) As you may understand I am not going to change the default value of protected attributes in a public API class and risk breaking any app that may have extended them and rely in the values being null, even if the documentation says that it is a boolean and does not mention it being nullable. Specially when the default values have been null instead of just false since they were introduced, so there might have been a good reason for that, and I am not going to touch that code unless I am fully confident that the change is correct.
But at least for this PR use
boolinstead of?boolin all places, it’s clearer.
Given that we do not know whether it is relevant or not to be able to differentiate if a value was ever set or not I would favour consistency with the existing code instead of clearness.
Given that we do not know whether it is relevant or not to be able to differentiate if a value was ever set or not I would favour consistency with the existing code instead of clearness.
Same
Is there something that blocks this still? I have an app that uses https://github.com/gruhn/vue-qrcode-reader, which loads a wasm file and the only way to get it to work is by setting $csp->allowEvalScript(true); at the moment which of course is not so nice. Having this PR in would allow to limit it to wasm files only.
Just missing a rebase and a second review :)
Rebased :-)