OurUmbraco icon indicating copy to clipboard operation
OurUmbraco copied to clipboard

Issue #275: Updated search to decode HTML strings before displaying

Open ed-parry opened this issue 6 years ago • 5 comments

Fixes issue #275 - I think the logic was wrong here - it should encode the strings on the way up, and decode them on the way down? So I've switched it over when rendering the search results, and removed the escaping step from the AJAX search drop-down as well, but shout if I'm miles off the mark here - assuming the text is encoded/escaped before being stored, so in the AJAX case we shouldn't need to escape again (and removing it allows the necessary characters to come through okay.

ed-parry avatar Jul 13 '18 18:07 ed-parry

@nul800sebastiaan Hey Sebastiaan - looks like a merge is needed here. Just wanted to check if (after resolving the conflicts) this would be good to go?

ed-parry avatar Sep 24 '18 19:09 ed-parry

Hey @ed-parry! I am actually unsure if we should do this, it could lead to people being able to craft some javascript to do nasty things. 🤔

nul800sebastiaan avatar Oct 18 '18 07:10 nul800sebastiaan

Morning @nul800sebastiaan. Yeah, I know what you mean. My thinking here is that the inputs are being checked and sanitized before going into the examine indexes, so it should be safe, but I'd be lying if I said I was sure. Can take another look to see if there's a better way, or at least test further with JS examples?

ed-parry avatar Oct 18 '18 07:10 ed-parry

Yeah I'm not sure if that is true, we do some sanitizing when people post things to the forum but I have also seen it break some things. I'd hate to break the whole search just because 1 forum post always comes up.

Although, the auto suggest search (see #275) gets the titles correct so maybe you can double check what we do there?

nul800sebastiaan avatar Oct 18 '18 08:10 nul800sebastiaan

Will do, leave it with me :)

ed-parry avatar Oct 18 '18 08:10 ed-parry