Crash with <a ping> and changing <base href>

RESOLVED FIXED in Firefox 48

Status

()

Core
Document Navigation
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jesse Ruderman, Assigned: ckerschb)

Tracking

(Blocks: 1 bug, {crash, regression, testcase})

Trunk
mozilla48
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox48 fixed)

Details

(crash signature)

Attachments

(4 attachments)

(Reporter)

Description

2 years ago
Created attachment 8731968 [details]
testcase

Testcase requires:
  user_pref("browser.send_pings", true);

Crashing line was added in:
  http://hg.mozilla.org/mozilla-central/rev/fa170734154a#l1.66
(Reporter)

Comment 1

2 years ago
Created attachment 8731969 [details]
stack

Comment 2

2 years ago
Christoph, could you take a look.
Flags: needinfo?(mozilla)
(Assignee)

Comment 3

2 years ago
Yeah, will fix that up.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(mozilla)
(Assignee)

Comment 4

2 years ago
Created attachment 8732369 [details] [diff] [review]
bug_1257730_fix_ping.patch

Thanks Jesse for the testcase.

Jonas, in the old version there was a null check for the uri here [1], we should make sure we don't deref a nullptr in the new version as well.

[1] http://hg.mozilla.org/mozilla-central/rev/fa170734154a#l1.15
Attachment #8732369 - Flags: review?(jonas)
(Assignee)

Updated

2 years ago
Blocks: 1192955
Keywords: regression
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Any reason we aren't landing the test as a crashtest too?
Flags: needinfo?(mozilla)
Flags: in-testsuite?
(Assignee)

Comment 6

2 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> Any reason we aren't landing the test as a crashtest too?

You are right. I'll convert the test into a crashtest and we should land that too. thanks!
Flags: needinfo?(mozilla)
Keywords: checkin-needed
(Assignee)

Comment 7

2 years ago
Created attachment 8732425 [details] [diff] [review]
bug_1257730_ping_crashtest.patch

Fails without the patch, but works correctly (no crash) with the patch.

Ryan, to speed things up I think it's fine if you review the test, right?
Flags: needinfo?(ryanvm)
Attachment #8732425 - Flags: review?(ryanvm)
Attachment #8732425 - Flags: review?(ryanvm) → review+
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/147b57e8c949
https://hg.mozilla.org/mozilla-central/rev/10d8bc81030b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.