504660 - EnsureMutable(0) doesn't?

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[[e-mail]]

EnsureMutable(0) calls SetLength(0) which calls SetCapacity(0) which sets mData to sEmptyBuffer which isn't mutable.

Would it possibly be more realistic to use ReplacePrep?

if (newLen == size_type(-1) || newLen == mLength)
  {
    if (mFlags & (F_FIXED | F_OWNED))
      return PR_TRUE;
    if ((mFlags & F_SHARED) && !nsStringBuffer::FromData(mData)->IsReadonly())
      return PR_TRUE;
  }
if (newLen < mLength)
  return ReplacePrep(0, newLen, mLength - newLen, 0);
return ReplacePrep(0, mLength, 0, newLen - mLength);

Comment 1

[Benjamin Smedberg]

EnsureMutable(0) doesn't actually need to do anything: you can't actually mutate a 0-length string... seems to me it works as intended.

Comment 2

[[e-mail]]

Some functions, such as nsUnescapeCount, overwrite a zero-length string...

Comment 3

[Benjamin Smedberg]

How does that work? Isn't that a buffer overflow?

Comment 4

[[e-mail]]

No, because they only want to overwrite it with another zero-length string.

Comment 5

[Benjamin Smedberg]

So specifically they write a null terminator? I rather dislike that, but if that's the case maybe we can just make EmptyString use a non-const buffer?

Comment 6

[[e-mail]]

Attachment: Like this?

Comment 7

[[e-mail]]

Pushed changeset d347a454b338 to mozilla-central.

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Seems like it's worth adding a test for this to TestStrings, since it's an odd use case.