413488 - nsACString::Find in nsStringAPI is buggy!

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[Prasad Sunkari [:prasad]]

Attachment: Fixes nsACString::Find function

Code from:
  http://lxr.mozilla.org/mozilla/source/xpcom/glue/nsStringAPI.cpp#710

710 nsACString::Find(const self_type& aStr, PRUint32 aOffset,
711                  ComparatorFunc c) const
712 {
713   const char_type *begin;
714   PRUint32 len = aStr.BeginReading(&begin);
715 
716   if (aOffset > len)
717     return -1;
718 
719   PRInt32 found = Find(begin + aOffset, len - aOffset, c);
720   if (found == -1)
721     return found;
722 
723   return found + aOffset;
724 }

The Find function above is supposed to find aStr starting at aOffset on this string.  But the code above seems to something completely wrong!

The other function Find being called inside this actually finds a "char *" of a given length in this string.

The attached patch fixes it and also makes minor (and silly) changes in a couple of other places.

Comment 1

[Benjamin Smedberg]

Comment on attachment 298460 [details] [diff] [review]
Fixes nsACString::Find function

This is ok, but it must have a unit test before landing.

Comment 2

[Benjamin Smedberg]

I think this should block beta4, as a serious error in the frozen string API.

Comment 3

[Mike Schroepfer]

Did this regress or is this old?

Comment 4

[Benjamin Smedberg]

This is a mismatch between the new frozen string API and the old nonfrozen one, so it's effectively a regression for extension authors.

Comment 5

[Reed Loden [:reed]]

As per comment #1, this needs a unit test before it can land.

Comment 6

[Prasad Sunkari [:prasad]]

Attachment: Adds testcases for Find function of nsStringAPI

This patch adds test cases for the Find function in nsStringAPI.  Apart from being a test case for this bug, it has test cases to test few other use cases of the Find functions too.

Comment 7

[Reed Loden [:reed]]

Checking in xpcom/glue/nsStringAPI.cpp;
/cvsroot/mozilla/xpcom/glue/nsStringAPI.cpp,v  <--  nsStringAPI.cpp
new revision: 3.11; previous revision: 3.10
done
Checking in xpcom/tests/TestStringAPI.cpp;
/cvsroot/mozilla/xpcom/tests/TestStringAPI.cpp,v  <--  TestStringAPI.cpp
new revision: 1.3; previous revision: 1.2
done