Closed Bug 236003 Opened 21 years ago Closed 20 years ago

null check missing (seamonkey version of bug 235129)

Categories

(SeaMonkey :: Bookmarks & History, defect)

Product:
SeaMonkey
Component:
Bookmarks & History
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
seamonkey1.0alpha

People

(Reporter: basic, Assigned: csthomas)

References

Details

Attachments

(2 files, 1 obsolete file)

patch for nsTSubstring::Equals (checked in)
1.39 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
patch
1.03 KB, patch
timeless
: review+
dbaron
: superreview+
asa
: approval1.8b2+
shaver
: approval1.8b3+
Details | Diff | Splinter Review
patch coming up
Attached patch patch (obsolete) — — Splinter Review
just got confirmation from aku on irc that this still crash on recent seamonkey trunk. upping severity.
Severity: normal → critical
Attachment #142577 - Flags: superreview?(brendan)
Comment on attachment 142577 [details] [diff] [review] patch Passing buck to darin to make sure this intended. Maybe neil can stamp r=? /be
Attachment #142577 - Flags: superreview?(darin)
Attachment #142577 - Flags: superreview?(brendan)
Attachment #142577 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 142577 [details] [diff] [review] patch Couldn't we initialize (line 4809) format to something non-null ("")?
The safest thing to do is to patch nsTSubstring::Equals(char * ... ) to support a NULL valued in-param. That might be a good plan for 1.7 final since we don't really want to take any chances, and it's very difficult to be sure that this is the only offender. This patch (or the one Neil is suggesting) seems like a good idea either way. I think if we add the NULL check to Equals, then we'd want to try removing it during the next Alpha cycle (especially when talkback is reporting!) ;-)
Attachment #143347 - Flags: review?(dbaron)
Comment on attachment 143347 [details] [diff] [review] patch for nsTSubstring::Equals (checked in) r=dbaron if you add an assertion or an NS_NOTREACHED so we at least assert in this case.
Attachment #143347 - Flags: review?(dbaron) → review+
attachment 143347 [details] [diff] [review] "nsTSubstring::Equals patch" has been checked in on the trunk.
*** Bug 236274 has been marked as a duplicate of this bug. ***
Comment on attachment 142577 [details] [diff] [review] patch sr=me for this patch or the one Neil suggested.
Attachment #142577 - Flags: superreview?(darin) → superreview+
Bug 241987 may be related to this one.
Product: Browser → Seamonkey
Attachment #142577 - Attachment is obsolete: true
Attachment #142577 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #143347 - Attachment description: patch for nsTSubstring::Equals → patch for nsTSubstring::Equals (checked in)
Assignee: p_ch → cst
Attached patch patch — — Splinter Review
Don't let it start out null; the code before the if() from the previous patch can't make it become null; a null check is no longer needed.
Attachment #180096 - Flags: superreview?(dbaron)
Attachment #180096 - Flags: review?(timeless)
Attachment #180096 - Flags: superreview?(dbaron) → superreview+
Whiteboard: [cst: r?]
Attachment #180096 - Flags: review?(timeless) → review+
Whiteboard: [cst: r?] → [cst: a?]
Comment on attachment 180096 [details] [diff] [review] patch a=asa
Attachment #180096 - Flags: approval1.8b2? → approval1.8b2+
Whiteboard: [cst: a?] → [cst: checkin]
Comment on attachment 180096 [details] [diff] [review] patch a=shaver
Attachment #180096 - Flags: approval1.8b3? → approval1.8b3+
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Whiteboard: [cst: checkin]
Target Milestone: --- → Seamonkey1.0alpha
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: