Prefixing a string to an empty autostring causes an extra pointless copy

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
String
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
mozilla1.9.2a1
Points:
---

Firefox Tracking Flags

(status1.9.1 .10-fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

8 years ago
Steps to reproduce problem:
1. Start a debug build
2. Turn off keyword.enabled
3. Try to load ://
Actual result: ###!!! ASSERTION: buffer incorrectly sized: 'a.Length() == headLen', file nsTSubstringTuple.cpp, line 78

What's happening here is that nsDefaultURIFixup strips off the bogus :// and attempts to prefix http:// in its place. It does this using

uriString.Assign(NS_LITERAL_CSTRING("http://") + uriString);

We create a substring tuple from "http://" and [R]uriString, and decide that [L]uriString doesn't depend on it, so that we can write to it in place. So we adjust [L]uriString's length, and then try to copy [R]uriString and "http://". Of course since [L]uriString and [R]uriString are the same thing, we've just changed [R]uriString's length which is why we get confused as to what to copy.
(Assignee)

Comment 1

8 years ago
We actually copy "http://" to uriString, and then uriString to uriString.
(Assignee)

Comment 2

8 years ago
Created attachment 362914 [details] [diff] [review]
Somewhat contrived test case

This test fails with our current code.
(Assignee)

Comment 3

8 years ago
Created attachment 362917 [details] [diff] [review]
Confident patch

Passes the test :-)
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #362917 - Flags: review?(benjamin)

Updated

8 years ago
Assignee: neil → nobody
Component: XPCOM → String
QA Contact: xpcom → string
(Assignee)

Updated

8 years ago
Assignee: nobody → neil
Attachment #362917 - Flags: review?(benjamin) → review+
(Assignee)

Comment 4

8 years ago
Pushed changeset 6c0ff682551d to mozilla-central.

Actually I pushed it yesterday. See bug 478732 for the gory details.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

7 years ago
Attachment #362917 - Flags: approval1.9.1.10?
would we not want this on the 1.9.2 branch if it's good for the 1.9.1 branch?
(Assignee)

Comment 7

7 years ago
It landed before 1.9.2 branched.
Comment on attachment 362917 [details] [diff] [review]
Confident patch

a=beltzner for 1.9.1.10
Attachment #362917 - Flags: approval1.9.1.10? → approval1.9.1.10+
(Assignee)

Comment 9

7 years ago
Pushed changeset 9a06693cdbb4 to releases/mozilla-1.9.1
status1.9.1: --- → .10-fixed
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.