Closed Bug 712212 Opened 8 years ago Closed 5 years ago

Popup summary after a failure has been starred needs to be html-escaped

Categories

(Tree Management Graveyard :: TBPL, defect, major)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: philor, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 694209 escaped the content of the summary in the annotated version which is displayed before the failure is starred, but as you can see from the J's in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=316d6a49a603 saying "Actual value 'Permission denied for to get property XPCComponents.classes'" rather than the correct "Actual value 'Permission denied for <file://> to get property XPCComponents.classes'", we don't escape once it's starred.
The Android M8's in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&onlyunstarred=1&rev=77a94dc888c9 are much more amusing, since they have an actual <input type="text"> in the middle of the popup.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Attachment #761225 - Flags: review?(philringnalda)
Comment on attachment 761225 [details] [diff] [review]
patch

oops, this breaks the unstarred case.
Attachment #761225 - Flags: review?(philringnalda)
I think this is right, but I haven't bothered to test it yet.

A more "correct" solution might be to change the MIME type to "text/html" for the annotated case, and have the client code escape (or not) based on the MIME type.  But that seems overly complicated for this bug.
Attachment #761225 - Attachment is obsolete: true
Attachment #761230 - Flags: review?(philringnalda)
Comment on attachment 761230 [details] [diff] [review]
patch v2 (untested)

Inconveniently, I don't seem to have a working tbpl install to test with.
Attachment #761230 - Flags: review?(philringnalda) → review?(emorley)
Comment on attachment 761230 [details] [diff] [review]
patch v2 (untested)

Unfortunately this breaks with:
[12-Jun-2013 12:28:40] PHP Warning:  htmlspecialchars() expects parameter 1 to be string, array given in /var/www/tbpl/php/getLogExcerpt.php on line 55
Attachment #761230 - Flags: review?(emorley) → review-
Not currently working on this.  If someone wants to fix my patch and test it, I think you just need to do something like array_map("htmlspecialchars", $rawErrorSummary).
Assignee: mbrubeck → nobody
Status: ASSIGNED → NEW
(In reply to Matt Brubeck (:mbrubeck) from comment #7)
> Not currently working on this.  If someone wants to fix my patch and test
> it, I think you just need to do something like array_map("htmlspecialchars",
> $rawErrorSummary).

CCing a few people who might be interested :-)
Also:
https://tbpl.mozilla.org/php/getParsedLog.php?id=42476171&tree=Mozilla-Inbound

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/content/base/test/browser_state_notifications.js | Found a tab after previous test timed out: data:text/html;charset=utf-8,<h1>1</h1>
Product: Webtools → Tree Management
Wontfix since TBPL is being replaced by Treeherder.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.