Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nextcloud does wrongly encode the Slogan #552

Closed
marbetschar opened this issue Dec 12, 2017 · 9 comments
Closed

Nextcloud does wrongly encode the Slogan #552

marbetschar opened this issue Dec 12, 2017 · 9 comments

Comments

@marbetschar
Copy link

We are able to define a slogan in the theming section of Nextcloud. Unfortunately this is displayed wrong to new users due to encoding errors.

Also, you can already see it's a bit broken when you set the slogan to 'foo & bar', and then just reload the settings page. It does not display & and instead displays & as the slogan.

1

2

3

MorrisJobke referenced this issue Dec 12, 2017
Slogan is already escaped in https://github.com/nextcloud/server/blob/cce4c285dbfd6957f50112b234b3545ebcceac54/apps/theming/lib/ThemingDefaults.php#L142

Fixes nextcloud/server#7460

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

Unfortunately this is displayed wrong to new users due to encoding errors.

Fix is in #51

Also, you can already see it's a bit broken when you set the slogan to 'foo & bar', and then just reload the settings page. It does not display & and instead displays & as the slogan.

I don't get this. What is the actual problem here? Could you rephrase it?

@marbetschar
Copy link
Author

@MorrisJobke thanks for the instant feedback. What I was trying to explain is:

If you enter foo & bar in the theme settings, save and refresh the page you'll notice there is a foo &amp; bar written in the input field. Probably due to the same encoding errors.

@MorrisJobke
Copy link
Member

If you enter foo & bar in the theme settings, save and refresh the page you'll notice there is a foo & bar written in the input field. Probably due to the same encoding errors.

cc @nextcloud/theming

@MorrisJobke
Copy link
Member

See comment by @blizzz and thus it needs more work.


Not convinced. getSlogan can be overridden by themes, for instance, we cannot be sure it always arrives sanitized. I'd rather remove the sanitation in ThemingDefaults. Also, apparently there is also a double escaping in the Login Controller as of now:

core/Controller/LoginController.php
193:            Util::addHeader('meta', ['property' => 'og:description', 'content' => Util::sanitizeHTML($this->defaults->getSlogan())]);

Requires more changes however, but I feel more comfortable when it is sanitized where the output takes place.

@juliusknorr
Copy link
Member

Requires more changes however, but I feel more comfortable when it is sanitized where the output takes place.

I cannot find the discussion, but I remember the argument for having this sanitized in the ThemingDefaults was, that we cannot ensure that apps always sanitize the output when they use values from the Defaults. From my POV that still would be fine, since the theming values can be changed by admins only.

@rnwgnr
Copy link

rnwgnr commented Feb 19, 2019

This is still an issue on my instance running on NextCloud 15.0.4:
When using special characters (like apostrophe or double quotes) in the slogan field, the FirstRunWizard shows the html entities despite the original character.
The login page handlles everything correct as shown by the opener.

See https://help.nextcloud.com/t/theming-slogan-encoding-issue/47915 for additional reference.

@szaimen szaimen transferred this issue from nextcloud/server Jun 24, 2021
@szaimen
Copy link
Collaborator

szaimen commented Jul 16, 2021

Maybe a fix like nextcloud/server#27912 would be possible here too?

@hatelamers
Copy link

hatelamers commented Nov 24, 2023

the issue still persists as of 24-11-2023 in Nextcloud 27.1.3: everywhere slogan is shown, it renders special characters incorrectly
' -> &#39;
" -> &quot;
& -> &amp;

@susnux
Copy link
Contributor

susnux commented Feb 27, 2024

This is not included in 28 and for 27 it works, see it shows "Foo & Bar":

image

@susnux susnux closed this as completed Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants