Closed
Bug 846458
Opened 12 years ago
Closed 12 years ago
intermittent TEST-UNEXPECTED-PASS | /tests/content/base/test/test_bug548193.html | Assertion count 0 is less than expected range 1-1 assertions.
Categories
(Core :: Security, defect)
Core
Security
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: dbaron, Assigned: imelven)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file, 1 obsolete file)
5.24 KB,
patch
|
geekboy
:
review+
|
Details | Diff | Splinter Review |
I think this is a rare enough intermittent unexpected pass that it's better off filed and starred than annotated (so we know when the assertion is fixed):
https://tbpl.mozilla.org/php/getParsedLog.php?id=20190512&tree=Mozilla-Inbound
Rev4 MacOSX Snow Leopard 10.6 mozilla-inbound debug test mochitest-1 on 2013-02-28 11:03:02 PST for push 67060725ec8d
slave: talos-r4-snow-054
11:08:27 INFO - 33536 ERROR TEST-UNEXPECTED-PASS | /tests/content/base/test/test_bug548193.html | Assertion count 0 is less than expected range 1-1 assertions.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
The test examines the upload stream in http-on-modify-request. It needs to rewind the stream before returning or this assertion fires.
What I don't understand is how come sometimes this assertion *doesn't* fire.
Assignee: nobody → imelven
Component: DOM → Security
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 59•12 years ago
|
||
Ian, have you had a chance to look at this yet? This is a top orange at the moment & I'd like to avoid just annotating this away if possible :-)
Flags: needinfo?(imelven)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 61•12 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #59)
> Ian, have you had a chance to look at this yet? This is a top orange at the
> moment & I'd like to avoid just annotating this away if possible :-)
Not yet, but I'll take a look today, thanks for the poke :)
Flags: needinfo?(imelven)
Assignee | ||
Comment 62•12 years ago
|
||
Pasting in some discussion from irc about khuey's initial investigation :
12:44 <@khuey> so what's happening is
12:44 <@khuey> the test hooks into http-on-modify-request
12:44 <@khuey> and looks at the upload stream
12:44 <@khuey> and reads from it
12:44 <@khuey> then later necko uploads it
12:45 <@khuey> and since the stream is seeked past 0 it asserts
12:45 <@khuey> that's why the assertion happens
12:45 < jduell> khuey: I'd think it's up to the sniffer to reseek to 0?
12:45 <@khuey> I don't understand why sometimes it *doesn't* happen
12:45 <@khuey> that seems to imply that http-on-modify-request and upload
starting are racing ...
12:46 < jduell> khuey: OMR is now called a bit later than it used to be
12:46 < jduell> it used to be called synchronously during asyncOpen
12:46 < jduell> Now it gets called after we've done some proxy resolution.
12:46 < jduell> khuey: We have a new on-opening-request notification that still
runs sync within asyncOpen
12:47 <@khuey> ah
12:49 < jduell> khuey: here's a start to the breadcrumb trail if you're
interested:
https://bugzilla.mozilla.org/show_bug.cgi?id=800799#c14
I don't immediately see a way to rewind the upload stream after reading from it. Maybe if we could copy it and read from the copy ? Alternately, we could switch to the new on-opening-request notification and see if that removes the race and the intermittant-ness - it sounds like it might. Is there a good way to test if that helps ?
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 65•12 years ago
|
||
Here is a patch that switches to using http-on-opening-request and removes the EnablePrivilege usage (replacing it with SpecialPowers) - this might be more consistent in always asserting about not rewinding the stream.
I tried seeking inputStream back to the beginning via QI'ing it to nsISeekableStream, but this then triggered another assertion : ###!!! ASSERTION: OnDataAvailable implementation consumed no data
Assignee | ||
Comment 66•12 years ago
|
||
I know what the OnDataAvailable assertion is about now, it's indeed a bug in the CSP code, patch coming shortly.
Assignee | ||
Comment 67•12 years ago
|
||
CSPViolationReportListener was ignoring the note on MDN about nsIStreamListener::OnDataAvailable saying that you have to read aCount bytes of data before returning - this patch does that as well as the other stuff from the previous patch.
I removed the SimpleTest.expectAssertions(1) and was going to pushed this to try to run m-1 for all platforms debug builds to have a look but the try tree is closed so that will have to wait :)
Attachment #725614 -
Attachment is obsolete: true
Attachment #725673 -
Flags: review?(sstamm)
Assignee | ||
Comment 68•12 years ago
|
||
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 83•12 years ago
|
||
Comment on attachment 725673 [details] [diff] [review]
patch v2
Review of attachment 725673 [details] [diff] [review]:
-----------------------------------------------------------------
Yeah, looks good to me. It's a bummer we have to create an instance of nsIScriptableInputStream instead of just futzing with the existing stream, but I can't see a better way. (Probably when we reimpl this in C++ we can avoid creating another stream obj.)
Attachment #725673 -
Flags: review?(sstamm) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 86•12 years ago
|
||
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 89•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment hidden (Legacy TBPL/Treeherder Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•