bug-2032278: escape user-provided field values for /signature graphs tab#7188
bug-2032278: escape user-provided field values for /signature graphs tab#7188biancadanforth merged 1 commit intomainfrom
Conversation
| $.each(lineDataObject, function (fieldValue, lineData) { | ||
| lineDataArray.push(lineData); | ||
| legend.push(key); | ||
| legend.push(escapeHTML(fieldValue)); |
There was a problem hiding this comment.
This is the one for the issue reported in the bug.
| show_secondary_x_label: false, | ||
| mouseover: function (d) { | ||
| $('.mg-active-datapoint', contentElement).html(d.term + ': ' + d.count + (d.count === 1 ? ' crash' : ' crashes')); | ||
| $('.mg-active-datapoint', contentElement).text(d.term + ': ' + d.count + (d.count === 1 ? ' crash' : ' crashes')); |
There was a problem hiding this comment.
This is another area I discovered while fixing the reported issue after I audited other places where we render field values outside of Django/Jinja (which autoescapes).
smarnach
left a comment
smarnach
left a comment
There was a problem hiding this comment.
This looks good to me. This is the right short-term solution for the problem.
I think the cause of this mistake is actually in the metrics-graphics package (specifically in the mg_add_legend_element() function, see the source code). This library apparently was originally developed at Mozilla, but the latest stable version is eight years old. We have a ticket to migrate to a different library somewhere. For our simple needs the D3 and metrics-graphics stacks seems total overkill anyway, so a migration to something simpler would be the right long-term solution.
2 participants
Because:
This PR:
How to test:
a. This was a little annoying, since the crash report is only in stage, and our bin/process_crashes.sh script defaults to prod. You can pass a
--hostflag, but then those args also get passed to thepubsub-clicommand which doesn't have that option and throws an error. This means the crash report was fetched and uploaded to the GCS emulator, but a message wasn't published to the local pubsub emulator. You can explicitly run the pubsub-cli command separately afterward as a workaround (pubsub-cli publish test local-standard-topic {$crash_id}).I have tested the fix locally.
Since we have no JS unit tests (see CRINGE-3), I didn't add any here.
Analogous Jira ticket: CRINGE-249