Last Comment Bug 479057 - Prefixing a string to an empty autostring causes an extra pointless copy
: Prefixing a string to an empty autostring causes an extra pointless copy
Product: Core
Classification: Components
Component: String (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla1.9.2a1
Assigned To:
: Nathan Froyd [:froydnj]
Depends on:
  Show dependency treegraph
Reported: 2009-02-18 08:30 PST by
Modified: 2010-04-05 14:24 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Somewhat contrived test case (984 bytes, patch)
2009-02-18 09:23 PST,
no flags Details | Diff | Splinter Review
Confident patch (1.60 KB, patch)
2009-02-18 09:34 PST,
benjamin: review+
mbeltzner: approval1.9.1.10+
Details | Diff | Splinter Review

Description User image 2009-02-18 08:30:59 PST
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.
Comment 1 User image 2009-02-18 08:36:53 PST
We actually copy "http://" to uriString, and then uriString to uriString.
Comment 2 User image 2009-02-18 09:23:37 PST
Created attachment 362914 [details] [diff] [review]
Somewhat contrived test case

This test fails with our current code.
Comment 3 User image 2009-02-18 09:34:15 PST
Created attachment 362917 [details] [diff] [review]
Confident patch

Passes the test :-)
Comment 4 User image 2009-03-05 01:50:16 PST
Pushed changeset 6c0ff682551d to mozilla-central.

Actually I pushed it yesterday. See bug 478732 for the gory details.
Comment 6 User image Daniel Veditz [:dveditz] 2010-03-29 10:37:19 PDT
would we not want this on the 1.9.2 branch if it's good for the 1.9.1 branch?
Comment 7 User image 2010-03-29 13:33:14 PDT
It landed before 1.9.2 branched.
Comment 8 User image Mike Beltzner [:beltzner, not reading bugmail] 2010-04-05 10:22:55 PDT
Comment on attachment 362917 [details] [diff] [review]
Confident patch

a=beltzner for
Comment 9 User image 2010-04-05 14:24:10 PDT
Pushed changeset 9a06693cdbb4 to releases/mozilla-1.9.1

Note You need to log in before you can comment on or make changes to this bug.