"ASSERTION: Item should only return null for out-of-bounds access" after explicitly adding null

RESOLVED FIXED in mozilla13

Status

()

Core
SVG
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: jwatt)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla13
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 595075 [details]
testcase

###!!! ASSERTION: Item should only return null for out-of-bounds access: 'PRUint32(aIndex) >= length', file dom/base/nsDOMClassInfo.cpp, line 10924

nsStringArraySH::GetProperty [dom/base/nsDOMClassInfo.cpp:10031]
XPC_WN_Helper_GetProperty [js/xpconnect/src/XPCWrappedNativeJSOps.cpp:963]
js::CallJSPropertyOp [js/src/jscntxtinlines.h:361]
js_NativeGetInline [js/src/jsscopeinlines.h:301]
js_GetPropertyHelperInline [js/src/jsobj.cpp:5396]
js::Interpret [js/src/jsobjinlines.h:209]

Suspicious: 7b0c5dcf5bcb (bug 722071), 2cd5af3aca9a (bug 529328)
(Assignee)

Updated

6 years ago
Assignee: nobody → jwatt
OS: Mac OS X → All
Hardware: x86_64 → All
(Assignee)

Comment 1

6 years ago
Created attachment 595412 [details] [diff] [review]
patch with tests
Attachment #595412 - Flags: review?(dholbert)
Comment on attachment 595412 [details] [diff] [review]
patch with tests

>+function run_tests()
>+{
>+  document.getElementById('svg').pauseAnimations();

Doesn't look like this ^ is necessary. (there are no animations in the test AFAICT)

>+  ok(initializeThrowsFor(strings, null),
>+     "SVGStringList.initialize() should throw when passed null");
>+  ok(initializeThrowsFor(strings, ""),
>+     "SVGStringList.initialize() should throw when passed the empty string");

As a sanity-check for the test, it'd probably be worth adding a call to each "xxxThrowsFor" function with a *nonempty* string, like so:

  ok(!initializeThrowsFor(strings, "ThisIsARealString"),
     "SVGStringList.initialize() shouldn't throw when passed a nonempty string");

to be sure that (a) the underlying methods don't just always throw (likely already covered elsewhere, but worth checking while you're here), and (b) your xxxThrowsFor test-methods don't just always return true due to a bug in the test.

Maybe these would belong at the end, so they don't muck up the rest of the test by modifying the string list?

Otherwise, r=me.

Also, should this bug be marked as security-sensitive?  (Comment 0 sounds scary, but the patch makes it look less scary.  Not sure what "Suspicious: [pointervalue]" meant in comment 0, but those are definitely non-null values, and I'd hope we aren't somehow pulling those out of IsEmpty strings & operating on them...)
Attachment #595412 - Flags: review?(dholbert) → review+
It's "Suspicious: [hg revision]", "Suspicious: [pointervalue]". There's only a correctness issue here.
Er, s$ion]", "Sus$ion]", not "Sus$
Ahhh, of course, sorry for my confusion.

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/16952c8a6b43
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.