Theming app caches complete URLs in APCu cache

[smueller18]

Steps to reproduce

1. Enable APCu as memcache.local
2. allow multiple trusted domains

Expected behaviour

Showing logo and background image an all trusted domains using memcache.

Actual behaviour

The first request of /apps/theming/styles?v=CACHE_BUSTER creates the following entry for the key UNIQUE_ID/theminggetScssVariables in the APCu cache:

Array
(
    [theming-cachebuster] => '24'
    [theming-logo-mime] => ''
    [theming-background-mime] => ''
    [image-logo] => 'https://MY_DOMAIN/core/img/logo.svg?v=CACHE_BUSTER'
    [image-login-background] => 'https://MY_DOMAIN/core/img/background.jpg?v=CACHE_BUSTER'
    [image-login-plain] => false
)

If a second tusted domain (MY_SECOND_DOMAIN) is called, the image-logo URL is loaded from the cache which shows to https://MY_DOMAIN/core/img/logo.svg?v=CACHE_BUSTER and therefore results in the following browser error:

Refused to load the image 'https://MY_DOMAIN/core/img/background.jpg?v=CACHE_BUSTER' because it violates the following Content Security Policy directive: "img-src 'self' data: blob:".

I inspected the nextcloud code a bit and think in this two lines the function $this->urlGenerator->getAbsoluteURL() has to be removed:

$variables['image-logo'] = "'".$this->getLogo()."'";
$variables['image-login-background'] = "'".$this->getBackground()."'";

I did this fix in my current installation and it works so far.
If you want, I can start a pull request.

Server configuration

Nextcloud version: 12.0.0

Nextcloud configuration:

Config report

...
'memcache.local' => '\\OC\\Memcache\\APCu',
...

Client configuration

Browser: Chrome Version 58.0.3029.110

[MorrisJobke]

@juliushaertl Haven't you worked on this recently?

[juliusknorr]

@MorrisJobke @smueller18 Yep, this is a duplicate of #5329 and has been fixed with #5429. It has also been backported to stable12.

[smueller18]

Ok, this fixes the issue. But why not save all URLs without base URL for css files? Then you wouldn't need separate cache entries for each trusted domain and therefore could reduce complexity. Or wouldn't it be backwards compatible?