Closed
Bug 335193
Opened 16 years ago
Closed 16 years ago
[FIX]URI fixup should not assume all NS_NewURI failures mean malformed URIs
Categories
(Core :: DOM: Navigation, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
Details
(Keywords: fixed1.8.1)
Attachments
(1 file, 2 obsolete files)
1.12 KB,
patch
|
Biesinger
:
review+
darin.moz
:
superreview+
darin.moz
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE: 1) Change nsViewSourceProtocol::NewURI to always throw 2) View source of any page EXPECTED RESULTS: Error message or error page or something ACTUAL RESULTS: Random web page loads in view-source window and starts sprouting popups. DETAILS: The page is what you get for "http://view-source". The problem is that URI fixup assumes any NS_NewURI failure means it needs to do fixup. It should really only be doing fixup if NS_NewURI returns NS_ERROR_MALFORMED_URI, imo.
![]() |
Assignee | |
Comment 1•16 years ago
|
||
Attachment #219559 -
Flags: superreview?(darin)
Attachment #219559 -
Flags: review?(cbiesinger)
Attachment #219559 -
Flags: approval-branch-1.8.1?(darin)
![]() |
Assignee | |
Updated•16 years ago
|
Priority: -- → P1
Summary: URI fixup should not assume all NS_NewURI failures mean malformed URIs → [FIX]URI fixup should not assume all NS_NewURI failures mean malformed URIs
Target Milestone: --- → mozilla1.9alpha
![]() |
Assignee | |
Comment 2•16 years ago
|
||
Attachment #219559 -
Attachment is obsolete: true
Attachment #219564 -
Flags: superreview?(darin)
Attachment #219564 -
Flags: review?(cbiesinger)
Attachment #219559 -
Flags: superreview?(darin)
Attachment #219559 -
Flags: review?(cbiesinger)
Attachment #219559 -
Flags: approval-branch-1.8.1?(darin)
![]() |
Assignee | |
Updated•16 years ago
|
Attachment #219564 -
Flags: approval-branch-1.8.1?(darin)
Comment 3•16 years ago
|
||
Comment on attachment 219564 [details] [diff] [review] Patch I meant to attach + if (!*aURI && rv != NS_ERROR_MALFORMED_URI) { + return rv; + } Hm... I'm not sure this is right. in the case where the NS_NewURI function above wasn't reached, couldn't uri be null but rv be success? why not move this inside the if?
![]() |
Assignee | |
Comment 4•16 years ago
|
||
Attachment #219564 -
Attachment is obsolete: true
Attachment #219638 -
Flags: superreview?(darin)
Attachment #219638 -
Flags: review?(cbiesinger)
Attachment #219638 -
Flags: approval-branch-1.8.1?(darin)
Attachment #219564 -
Flags: superreview?(darin)
Attachment #219564 -
Flags: review?(cbiesinger)
Attachment #219564 -
Flags: approval-branch-1.8.1?(darin)
Comment 5•16 years ago
|
||
Comment on attachment 219638 [details] [diff] [review] Good catch Didn't we add a call to NS_NewURI prior to calling into CreateFixupURI? Does that need to change?
Attachment #219638 -
Flags: superreview?(darin)
Attachment #219638 -
Flags: superreview+
Attachment #219638 -
Flags: approval-branch-1.8.1?(darin)
Attachment #219638 -
Flags: approval-branch-1.8.1+
Updated•16 years ago
|
Attachment #219638 -
Flags: review?(cbiesinger) → review+
![]() |
Assignee | |
Comment 6•16 years ago
|
||
That call is just used to decide whether to do Google searches.
![]() |
Assignee | |
Comment 7•16 years ago
|
||
Fixed, trunk and 1.8 branch.
You need to log in
before you can comment on or make changes to this bug.
Description
•