Closed Bug 203384 Opened 21 years ago Closed 21 years ago

Crash when resolving xpath1() XPointer scheme

Categories

(Core :: XML, defect)

Product:
Core
Component:
XML
Platform:
x86
Windows 2000
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.4beta

People

(Reporter: hjtoi-bugzilla, Assigned: sicking)

References

Details

(Keywords: crash, regression)

Attachments

(1 file, 1 obsolete file)

Do the right thing when the document is missing
1.15 KB, patch
peterv
: review+
hjtoi-bugzilla
: superreview+
asa
: approval1.4+
Details | Diff | Splinter Review 
To reproduce, open file mozilla/content/xml/tests/xpointer/xpointer.xml, and
click on the "JS" button one line that begins with "xpath1".

What happens: We crash in here because |doc| is null:

nsXPathEvaluator::CreateExpression(const nsAString & aExpression,
                                   nsIDOMXPathNSResolver *aResolver,
                                   nsIDOMXPathExpression **aResult)
{
    nsCOMPtr<nsIDocument> doc = do_QueryReferent(mDocument);
    ParseContextImpl pContext(aResolver, doc->IsCaseSensitive()); // <= CRASH

mDocument is also null and it is not set anywhere, so this seems kind of strange. 
This piece of code was last changed by jst in bug 111514. Maybe something else
changed around here recently as well?
Attached patch Proposed patch from jst (obsolete) — — Splinter Review 
jst emailed me this patch, and it seems to work
Attachment #121714 - Flags: superreview+
Attachment #121714 - Flags: review?(bugmail)
Flags: blocking1.4b?
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla1.4beta
Attachment #121714 - Flags: review?(bugmail) → review+
*** Bug 203762 has been marked as a duplicate of this bug. ***
Comment on attachment 121714 [details] [diff] [review]
Proposed patch from jst

Lowrisk patch which fixes a crash.
Attachment #121714 - Flags: approval1.4b?
Comment on attachment 121714 [details] [diff] [review]
Proposed patch from jst

a=asa (on behalf of drivers) for checkin to 1.4b
Attachment #121714 - Flags: approval1.4b? → approval1.4b+
Checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Whiteboard: [HAVE FIX]
Flags: blocking1.4b?
ouch. Sorry guys, this does the wrong thing when the document is missing, my
bad. Patch comming in a sec
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
-> me
Assignee: jst → bugmail
Status: REOPENED → NEW
If there is no document (which is compleatly valid) we should be case sensitive
Attachment #121714 - Attachment is obsolete: true
Attachment #123017 - Flags: superreview?(jst)
Attachment #123017 - Flags: review?(peterv)
Comment on attachment 123017 [details] [diff] [review]
Do the right thing when the document is missing

Fine by me
Attachment #123017 - Flags: review?(peterv) → review+
Attachment #123017 - Flags: superreview?(jst) → superreview+
Comment on attachment 123017 [details] [diff] [review]
Do the right thing when the document is missing

this patch is very safe and only affects uses of DOM-XPath and xpointer.
Without the patch xpointer will have trouble with casesensitivity
Attachment #123017 - Flags: approval1.4+
Comment on attachment 123017 [details] [diff] [review]
Do the right thing when the document is missing

opps, i ment to _request_ approval of course, nothing else
Attachment #123017 - Flags: approval1.4+ → approval1.4?
Comment on attachment 123017 [details] [diff] [review]
Do the right thing when the document is missing

a=asa (on behalf of drivers) for checkin to 1.4.
Attachment #123017 - Flags: approval1.4? → approval1.4+
checked in
Status: NEW → RESOLVED
Closed: 21 years ago → 21 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: