-
Notifications
You must be signed in to change notification settings - Fork 554
[Bug] Search result descriptions are double-escaped #1239
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
Comments
Search for |
@lhsazevedo, is this escape needed? - ${escape(description)}
+ ${description} |
@lhsazevedo, taking a look at implementation of |
Yup, but only for HTML entities. Decoding with an |
By the way, we only need to be this safe because we use const el = document.createElement('a');
el.href = url;
el.className = 'search-modal__result';
el.setAttribute('role', 'option');
el.setAttribute('aria-labelledby', `search-modal__result-name-${i}`);
el.setAttribute('aria-describedby', `search-modal__result-description-${i}`);
el.setAttribute('aria-selected', 'false');
const iconEl = document.createElement('div');
iconEl.className = 'search-modal__result-icon';
iconEl.innerHTML = icon; // icon is trusted and safe
const contentEl = document.createElement('div');
contentEl.className = 'search-modal__result-content';
const nameEl = document.createElement('div');
nameEl.className = 'search-modal__result-name';
nameEl.id = `search-modal__result-name-${i}`;
nameEl.textContent = item.name; // use textContent for untrusted content
const descEl = document.createElement('div');
descEl.className = 'search-modal__result-description';
descEl.id = `search-modal__result-description-${i}`;
descEl.textContent = description; // use textContent for untrusted content
contentEl.append(nameEl, descEl);
el.append(iconEl, contentEl);
resultsElement.appendChild(el); This is quite imperative though, which makes it harder to interpret the markup structure. resultsElement.append(
a(
{
href: url,
className: "search-modal__result",
role: "option",
ariaLabelledby: `search-modal__result-name-${i}`,
ariaDescribedby: `search-modal__result-description-${i}`,
ariaSelected: "false",
},
[
div({ className: "search-modal__result-icon", innerHTML: icon }),
div({ className: "search-modal__result-content" }, [
div(
{
id: `search-modal__result-name-${i}`,
className: "search-modal__result-name",
},
item.name,
),
div(
{
id: `search-modal__result-description-${i}`,
className: "search-modal__result-description",
},
description,
),
]),
],
),
); While I'm Ok with this approach, it might be a bit of over-engineering... |
Search content is generated by phd based on |
Even then, I'd still vote for escaping it, since we don't want to render unexpected markup. |
Here is what happens when searching for an enum in the French version of the website (using French is relevant because the French description contains accents):
The text was updated successfully, but these errors were encountered: