Less-than ("<") characters are not saved in collection descriptions. Greater-than (">") characters are saved, however not if they are preceded by their counterpart. i.e. "<<>>>" saves as ">". Clearly something isn't being escaped correctly here.
In fact, do something like "<asdgasgd>" and none of it will be saved. See also bug 498080 & bug 507834.
Summary: Collection description not properly escaped; cannot add a less-than "<" to description → Collection description not properly escaped; less-than "<" and characters after it are not saved
This is not only for collection descriptions, but also for many other user-provided data on AMO. See bug 493247 comment 2.
(In reply to comment #0) > Less-than ("<") characters are not saved in collection descriptions. > Greater-than (">") characters are saved, however not if they are preceded by > their counterpart. i.e. "<<>>>" saves as ">". Clearly something isn't being > escaped correctly here. This might be merely a difference of terminology, but I do not think this is a matter of escaping, but this is a matter of so-called “sanitizing.” It seems that in many cases, AMO “sanitizes” input assuming incorrectly that the input is meant to be HTML; that is, it assumes that “<...>” in the input is meant to be an HTML tag and removes it. This prevents a user from entering angle brackets even when they are meant to be simply angle brackets, not an HTML tag. A better way is to treat input as plaintext and escape “<” to “<” and so on when generating HTML (that is, when storing it to the database if the values in the database are HTML, or when producing the output for the user if the values in the database are plaintext).
I noticed bug 343573. Guessing from that bug, maybe collection descriptions (as well as the content of many of the other text fields) are meant to be HTML, and the support for tags has merely not been implemented yet. If this is the case, removing “<<>>” might be correct because it is not correct as HTML. In that case, we should be able to write “<<>>” to represent that string, but unfortunately, this has not been implemented, either.
The reason is that the Amo->clean() method calls php's strip_tag, which removes anything that is even remotely possible to being a HTML/PHP tag. We already sanitize the data when we output. So I think it's safe to tell the clean() method to skip calling strip_tags, and just save (possible) HTML in the DB.
Assignee: nobody → cdolivei.bugzilla
Status: NEW → ASSIGNED
Attachment #399665 - Flags: review?(rdoherty)
Cesar: You think you could also apply the same quick fix for bug 493247? There's probably some other fields that do this too.
Attachment #399665 - Flags: review?(rdoherty) → review-
Comment on attachment 399665 [details] [diff] [review] don't strip the tags This works when I edit a collection, but not when I'm creating a new one. Characters are stripped on creation.
oops. my bad. thanks for catching that!
Comment on attachment 400411 [details] [diff] [review] oops Looks good!
Attachment #400411 - Flags: review?(rdoherty) → review+
Today is code freeze, not waiting for a commit anymore. :) r51811 Thanks cesar/ryan.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Verified FIXED on https://preview.addons.mozilla.org/en-US/firefox/collections/edit/8de4c068-13d0-94ed-ae52-fc9550c6bcce; I tested both creating and editing a collection with < and > in its description.
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.