SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.95k stars 1.23k forks source link

`sap.ui.layout` seemingly does not comply with CSP if the `unsave-eval` directive is removed #3727

Closed dfenerski closed 1 year ago

dfenerski commented 1 year ago

OpenUI5 version: 1.108.7

Browser/version (+device/version): CR latest

Steps to reproduce the problem:

  1. Remove unsave-eval from CSP header on the server
  2. Open DEV tools
  3. Open app which uses preloaded sap.ui.layout

What is the expected result? The app should work

What happens instead? The app fails to start image

Any other information? (attach screenshot if possible) The table here does not list sap.ui.layout as incompatible library. Am I missing something or doing something wrong with regards to the build?

Thanks in advance & BR

codeworrior commented 1 year ago

Hi @dfenerski,

this is IMO not an issue of sap.ui.layout, but of the code which loads the library. There seems to be a sync loading call somewhere. This can either be an explicit Core.loadLibrary("sap.ui.layout") call or a component which is loaded in a not fully-asynchronous way (e.g. without "manifest first").

For a further analysis, can you please provide more details about the context in which the error occurs?

dfenerski commented 1 year ago

I am getting this warning during build of one of the libraries that the app consumes image It is a web worker used for some background tasks, loaded in the app trough image

However removing the file and it's loading did not change the error message.

dfenerski commented 1 year ago

Both the app and the library are written in TypeScript as per the official guides. I am also getting the following subsequent error (regarding the library?) image The library.ts itself contains basically 2 instructions - image and image

codeworrior commented 1 year ago

But this is now not the sap.ui.layout library.ts (which shouldn't exist as we don't provide TS sources)?

I'm still missing a hint who's loading the sap.ui.layout library and how. Could you please check in the network tab of the dev tools who the "initiator" of the request for sap/ui/layout/library-preload.js is? ui5loader and Lib.js/Core.js are of no interest, rather the call stack above, calling those core APIs.

Side remark reg. the worker code: worker code should be excluded from the library-preload bundle as it anyhow won't be loaded via the ui5loader. But as long as the ui5loader doesn't load it, this shouldn't cause CSP issues.

dfenerski commented 1 year ago

Apologies, that was the library.ts of an internal library which I thought might be relevant. sap.ui.layout is listed as dependency of this internal TS library in the ui5.yaml file.

Full network trace: image

Require stack for library-preload of sap.ui.layout: image

Interestingly the 'non-preload' version of the file gets requested too; it's require stack is very similar. IDK if this is an error.

dfenerski commented 1 year ago

Bootstrap tag, manifest look like this image image

App uses v3 tooling, sap.ui.layout is listed as dependency in the ui5.yaml file; minify task ran successfully and I can see minified (preload) files for both app and TS lib

dfenerski commented 1 year ago

This member of the call stack image does indeed correspond to the transpiled from TS initLibrary call: image Traversing the stack a bit, the next interesting thing that gets executed is this line with warning comment: image After this, sync preload / imports begin to happen.

codeworrior commented 1 year ago

Thank you, this helps indeed.

From the initiator call stack, I can see that Core#initLibrary is the one that loads the sap.ui.layout lib synchronously. This call in Core#initLibrary serves as a legacy fallback, if none of the other means to load dependency libraries has worked out. As Core#initLibrary is a sync API that guarantees that dependencies have been loaded on return, there's no choice but to use sync loading in that place (not CSP compliant).

Ideally, dependency libraries should have been loaded already by the framework before the code in library.js/.ts is executed. This is controlled by dependency information either in the manifest.json of the library or in the .library file (if the latter exists) When there's no manifest.json, but a .library, then the ui5 tooling generates the manifest.json during build time from the .library.

Samples how the dependencies have to be maintained can be seen in the sap.ui.table library:

I'm unfortunately not up-to-date with all the samples/documentation that we have. So I can't say out of the box whether this important information is missing in samples or whether it can be seen somewhere. If you name your source of information, I can check whether it needs to be enriched :-)

codeworrior commented 1 year ago

Think about it again, as a first step, you just might add an AMD dependency to sap/ui/layout/library to your library.ts file. This should already prevent the sync loading in the ui5 serve scenario. For the full-build scenario, it's still better to have the dependency finally in the manifest.json.

dfenerski commented 1 year ago

Thanks for the info!

I have followed the official 'TS' + 'UI5 library' guide here. My lib does have a .library file, but perhaps manifest generation does not kick in due to the initLibrary call.. I know this is considered WIP but there were no known functionality limitations to my understanding, would be a bit sad to see one now.

Unsure how to proceed, is there a way to make this work? Any help / hints / workarounds on this are greatly appreciated. I could try to get my hands dirty with this, but I'd consider it a bit dangerous and may consume a lot of time & yield no results or workarounds that are not officially endorsed..

The .library file of the TS lib:
image

codeworrior commented 1 year ago

Please give my last proposal with the AMD dependency a try (https://github.com/SAP/openui5/issues/3727#issuecomment-1491855132) and let us know the results.

Reg. samples / documentation: Internally, we have the following documentation which we maybe should make available externally as well (as far as it is applicable). We're working on cleaning up this jungle a bit, but like so often, compatibility makes it harder to get rid of any of the locations.

Where to Maintain Library Dependencies

There are 6 locations where library dependencies are maintained

The following table shows where and how to maintain what

Location Eager Lazy Comment
Dot library All All, with nested  true element  
Library.js All None The initLibrary call should list all eager dependencies in the dependencies property.Lazy libraries must not be listed in the initLibrary Call (they wouldn’t be lazy then)The AMD dependencies must list all library.js modules for those libraries that are mentioned as a dependency in the initLibrary call
~Pom.xml~ ~All~ ~All, as long as this doesn’t create cycles. Eager dependencies must never create cycles. They have to be avoided completely. If lazy dependencies would cause cycles, they could be omitted. But so far we managed to avoid this case and we would prefer to continue with that practice as it has some advantages reg. testing etc.~
ui5.yaml All All, as long as this doesn't create cycles. With the current concepts for DIST layer development, the ui5.yaml does not contain dependency information. Only for apps or libs on top of SAPUI5 See (internal link removed)
Package.json -- -- See (internal link removed)
manifest.json -- -- For all SAPUI5 libs, the manifest is created from the information contained in the .library file. So nothing has to be maintained, the file will be updated with each build
dfenerski commented 1 year ago

Many thanks, codeworrior, it worked!

The library.ts file now looks like this: image and gets transpiled to image

I will test a bit more, but I believe the issue can be closed. It was off-topic either way - it has more to do with the TS guide.

Should I open it there?

BR