search

martin-g / wicket-bootstrap

Apache Wicket components for Twitter Bootstrap - Wicket-Bootstrap is based on Twitter's toolkit (bootstrap) and the Apache Wicket Framework.
https://wicketbootstrap.teliclab.info/
296 stars 162 forks source link

Make `BootstrapFileInputField` strict content security policy compliant #901

Open nmandrescu opened 3 years ago

nmandrescu commented 3 years ago

When loading a BootstrapFileInputField that has no errors, it is hidden through style attribute

<div class="kv-fileinput-error file-error-message" style="display: none;"></div>

It is happening in Wicket Bootstrap 5.0.4 https://github.com/l0rdn1kk0n/wicket-bootstrap/blob/add49f25467d2f7ee51a5aa88eff308c6b302fb8/bootstrap-extensions/src/main/java/de/agilecoders/wicket/extensions/markup/html/bootstrap/form/fileinput/res/js/fileinput.js#L661

Below is another .hide() call and likely more similar cases exists.

Adding style attribute is not allowed in CSP strict mode.

Please fix BootstrapFileInputField and other components to be CSP compliant.

martin-g commented 3 years ago

Thanks for the report! Pull Requests are welcome!

reckart commented 1 year ago

Considering this is Bootstrap we are using here, I believe the change should be simple from switching to add a style attribute to adding/removing the Bootstrap CSS class d-none instead.

reckart commented 1 year ago

However, this is not a thing that should be fixed in Wicket Bootstrap because it would prevent updating the fileinput component which originally comes from https://github.com/kartik-v/bootstrap-fileinput. So this should probably be fixed upstream in https://github.com/kartik-v/bootstrap-fileinput.

reckart commented 1 year ago

Essentially, the CSP issue boils down to calls to JQuery's parseHTML and innerHTML which are used by the Bootstrap FileInput. I cannot even see that calls to .hide() are an issue at this point. Unfortunately, this is something not easy to fix, even upstream in https://github.com/kartik-v/bootstrap-fileinput because it would probably mean that the whole templating and even theming approach might need to be reimplemented...

reckart commented 1 year ago

There was an upstream issue related to CSP which actually did re-implement parts of the templating/theming: https://github.com/kartik-v/bootstrap-fileinput/issues/1565

However, I just checked in the latest version of bootstrap-fileinput and it still uses parseHTML and innerHTML which triggers a CSP error for me in Chrome when not using the unsafe-inline policy.

reckart commented 1 year ago

I have opened another upstream issue: https://github.com/kartik-v/bootstrap-fileinput/issues/1833