Edit a label, get " back: the HTML-entity round-trip bug

A coach names a nav link Coach's "A" Team, saves it, and it renders fine. Then they open the edit dialog to fix a typo and the textbox reads Coach's "A" Team. The stored data is correct. The edit form is showing them entity gibberish, and if they save over it, the gibberish becomes the new label.

This is one of those bugs that looks like a display glitch and is actually a data-integrity boundary problem. Here is the whole thing, including the one ordering detail that bites you if you fix it carelessly.

The setup: store an escaped form, render it back to characters

The site’s nav banners let a site admin name and link the items in the top navigation. Labels are user-supplied free text, so they can contain anything a coach types: quotes, apostrophes, ampersands, the occasional <. The stored form is escaped so it survives both the serialized link blob and the rendered HTML. Quotes in particular get round-tripped through placeholder tokens: a DisplayQuotes() helper turns the stored [quote] token into &quot; (and [squote] back into a literal apostrophe) when it builds the in-memory link object, and the entity renders correctly in the nav. That part works. Escaping untrusted text before it lands in HTML is exactly what you are supposed to do.

The problem is what happens when you load that same value back into an editable field.

The bug: the edit form populated the input with the escaped string

The edit dialog filled the label textbox straight from the in-memory link object:

$("#propertyLabel").val(linkObj.label)

linkObj.label already carries &quot; for every quote, because DisplayQuotes() put it there. So the input box shows Coach's &quot;A&quot; Team instead of Coach's "A" Team. An <input> value is plain text, not HTML. It does not decode entities. It just shows you the literal characters, ampersand and all.

Now follow the round-trip. The coach did not type &quot;, so as far as they are concerned the field is showing garbage. The onkeyup handler on that input, PropertyLabelUpdate(this.value), writes whatever is in the box straight back to linkObj.label. So the moment they touch the field and save, the literal text &quot; becomes the label. On the next load that escaped string runs through the display path again, and the & in &quot; gets re-escaped to &amp;. Save through the dialog again and you compound it further. Each edit through the buggy dialog can add another layer of escaping.

That is the failure mode the decode is there to prevent: an editable field that shows the escaped form, accepts it back verbatim, and lets the entities pile up &amp;quot;, &amp;amp;quot;, one careless save at a time. The fix stops it at the source by never letting the escaped form reach the input in the first place.

The fix: decode before you put it in the input

The fix is to turn the entities back into characters before the value goes into the textbox, so the input shows what the coach actually typed. In NavBanners.js, inside PropertySectionUpdate, that meant mapping the entities a label can contain back to their characters before the .val() call:

// use a regex to replace all html entities with their actual characters
var label = linkObj.label.replace(/&quot;/g, '"');
label = label.replace(/&amp;/g, '&');
label = label.replace(/&lt;/g, '<');
label = label.replace(/&gt;/g, '>');
label = label.replace(/&quot;/g, '"');
label = label.replace(/&apos;/g, "'");
label = label.replace(/&nbsp;/g, " ");
label = label.replace(/&copy;/g, "©");


$("#propertyLabel").val(label);

Now the box shows Coach's "A" Team, the coach edits real text, and the round-trip stops compounding. (Yes, &quot; is in there twice. That is the literal diff, not a tidied-up version.)

The detail that will burn you on arbitrary input: where &amp; goes in the chain

There is a trap in any decode-by-chained-replace and it is worth slowing down for, because the fix above does not actually avoid it. It gets away with it. The order you decode entities in matters, because &amp; is the entity for the ampersand and every other entity starts with an ampersand.

Consider a stored value of &amp;lt;, which is the escaped form of the literal text &lt; (someone typed the four characters ampersand-l-t-semicolon and wanted them to survive). If you decode &amp; to & early, you turn &amp;lt; into &lt;. Then your &lt; rule fires on that result and turns it into <. You just decoded a layer that was not yours to decode, and the user’s literal &lt; text became a < character. Run that through the save path and the data is now wrong in the other direction.

The general rule for arbitrary input: when you decode a set of entities by chained string replacement, handle &amp; last, not in the middle. Decode all the named entities while the ampersands are still escaped, so a &amp; that is protecting some user’s literal entity text does not get unwrapped early and let the next rule chew through it. Decode &amp; only after everything else, when any remaining &amp; genuinely means “the user wanted an ampersand here.”

The same hazard exists on the encode side in reverse: when you escape, you escape & first, before <, >, and the rest, so you do not double-escape the ampersands you just introduced. Escape & first, decode & last. The ampersand is special on both sides and its position in the chain is not arbitrary.

Now look at the actual fix. It decodes &amp; second, right after the first &quot;. That violates the rule I just gave you. It is fine here, and that is the honest interesting part: for this bounded set of labels, the entities that can realistically show up are quotes, apostrophes, and the odd ampersand, not nested constructions like &amp;lt;. None of the other handled entities can be produced by an early &amp; unwrap in a way that matters for these inputs, so the early &amp; decode never bites. If you copy this chain into something that takes arbitrary input, move the &amp; decode to the end. The bounded version works on luck you do not have in the general case.

The real lesson: every editable field is a decode/encode boundary

The bug is not “we forgot to decode in one place.” The bug is treating the edit form as if it were just another display surface. It is not. A render target and an <input> have opposite requirements even though they hold the same logical value.

  • The render path wants the value HTML-encoded, because it is dropping text into HTML.
  • The input path wants the value decoded, because an <input> holds plain text and will show entities literally.
  • The save path wants to encode exactly once, on the way in.

Wire those three correctly and the value survives any number of round-trips. Get the input path wrong, populate the box with the escaped string, and the next save writes the escaped text back as if the user had typed it, because from the form’s point of view they did. Escaping on the way in is correct. The failure was forgetting that the way out of storage into an editable field is itself a boundary that needs the inverse transform.

If you store an escaped form, then for every editable surface you need three things in agreement: decode when you load the value into the field, edit the decoded text, and re-escape once when you save. Miss the decode and you do not get an error. You get a slow accretion of &amp;quot;, then &amp;amp;quot;, that nobody notices until a coach’s nav link is several layers deep and the only way out is a manual cleanup.

Audit your edit forms. Anywhere you call .val(storedValue) on a field whose source is an escaped string, you have this bug waiting. It will not throw. It will just quietly fold your users’ data over on itself, one well-intentioned save at a time.