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

Fix double escaping of slogan #51

Closed
wants to merge 1 commit into from
Closed

Conversation

MorrisJobke
Copy link
Member

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>
@blizzz
Copy link
Member

blizzz commented Dec 12, 2017

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.

@MorrisJobke MorrisJobke deleted the fix-double-escaping branch December 12, 2017 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants