CalculateUTF8Length increases reading iterator beyond end of string




17 years ago
17 years ago


(Reporter: Hixie (not reading bugmail), Assigned: jag (Peter Annema))



Firefox Tracking Flags

(Not tracked)



(2 attachments)



17 years ago
While trying to open my INBOX, mail news locked up with the stack given below.

Note that this code only gets invoked in debug mode (see #7):

   277 #if DEBUG
   278 (void) NS_ConvertUTF8toUCS2(line).get(); // asserts if invalid UTF-8
   279 #endif

However it appears it does worse than assert. :-) Of course, just removing this 
code is not the right fix, since NS_ConvertUTF8toUCS2.get() should never 
lock up, whatever the input...

#0 0x40636625 in fflush () at

#1  0x401df868 in nsDebug::Assertion ( aStr=0x4024e700 "Infinite loop:
    can't advance a reading iterator beyond the end of a string",
    aExpr=0x4024e6ed "one_hop>0", aFile=0x4024e6c0
    "../../dist/include/string/nsStringIterator.h", aLine=174) at

#2  0x40234898 in nsReadingIterator<char>::advance (this=0xbfffedf8,
    n=37) at ../../dist/include/string/nsStringIterator.h:174

#3  0x40235173 in nsCharSourceTraits<nsReadingIterator<char>
    >::advance (s=@0xbfffedf8, n=65) at

#4  0x4023b9d9 in CalculateUTF8Length &
    copy_string<nsReadingIterator<char>, CalculateUTF8Length> (
    first=@0xbfffedf8, last=@0xbfffedd8, result=@0xbfffedd4) at
    0x40211ca9 in NS_ConvertUTF8toUCS2::Init (this=0xbfffee98,
    aCString=@0xbfffee38) at nsString2.cpp:1705

#6  0x42bae239 in NS_ConvertUTF8toUCS2::NS_ConvertUTF8toUCS2
    (this=0xbfffee98, aCString=0x83bf308 "
    (Jeandré)") at ../../../dist/include/string/nsString2.h:598

#7  0x42ba1dae in nsMsgHeaderParser::ParseHeaderAddresses
    (this=0x88c18e0, charset=0xbffff250 "ISO-8859-1", line=0x83bf308
    " (Jeandré)", names=0xbffff04c,
    addresses=0xbffff048, numAddresses=0xbffff040) at

#8  0x42a6f6c4 in nsMsgSearchTerm::MatchRfc822String (this=0x89215b0,
    string=0x83bf308 " (Jeandré)",
    charset=0xbffff250 "ISO-8859-1", charsetOverride=0,
    pResult=0xbffff158) at nsMsgSearchTerm.cpp:966

#9  0x42a72fc5 in nsMsgSearchOfflineMail::MatchTerms
    (msgToMatch=0x895cd60, termList=0x8921570,
    defaultCharset=0xbffff250 "ISO-8859-1", scope=0x8679d40,
    db=0x86729e8, headers=0x8a93410 "Return-Path:
    <>", headerSize=1383,
    Filtering=1, pResult=0xbffff308) at nsMsgLocalSearch.cpp:571

#10 0x42a72b75 in nsMsgSearchOfflineMail::MatchTermsForFilter
    (msgToMatch=0x895cd60, termList=0x8921570,
    defaultCharset=0xbffff250 "ISO-8859-1", scope=0x8679d40,
    db=0x86729e8, headers=0x8a93410 "Return-Path:
    <>", headerSize=1383,
    pResult=0xbffff308) at nsMsgLocalSearch.cpp:500

#11 0x42a6bdad in nsMsgFilter::MatchHdr (this=0x89214c0,
    msgHdr=0x895cd60, folder=0x85446bc, db=0x86729e8,
    headers=0x8a93410 "Return-Path:
    <>", headersSize=1383,
    pResult=0xbffff308) at nsMsgFilter.cpp:355

#12 0x42a68006 in nsMsgFilterList::ApplyFiltersToHdr (this=0x8a8eae0,
    filterType=1, msgHdr=0x895cd60, folder=0x85446bc, db=0x86729e8,
    headers=0x8a93410 "Return-Path:
    <>", headersSize=1383,
    listener=0x8544790, msgWindow=0x88e4e70) at

#13 0x423b86b0 in nsImapMailFolder::NormalEndHeaderParseStream
    (this=0x85446a0, aProtocol=0x89f1cc0) at nsImapMailFolder.cpp:2520

#14 0x401f4278 in XPTC_InvokeByIndex (that=0x854477c, methodIndex=18,
    paramCount=1, params=0x8294990) at xptcinvoke_unixish_x86.cpp:153

#15 0x401d83eb in EventHandler (self=0x89b2c38) at

#16 0x401cfdfc in PL_HandleEvent (self=0x89b2c38) at plevent.c:590

#17 0x401cfbe9 in PL_ProcessPendingEvents (self=0x80a75d8) at plevent.c:520

#18 0x401d2060 in nsEventQueueImpl::ProcessPendingEvents
    (this=0x80a75b0) at nsEventQueue.cpp:389

#19 0x40d0c744 in event_processor_callback (data=0x80a75b0, source=4,
    condition=GDK_INPUT_READ) at nsAppShell.cpp:184

#20 0x40d0c2c3 in our_gdk_io_invoke (source=0x814a780,
    condition=G_IO_IN, data=0x814a770) at nsAppShell.cpp:77

Comment 1

17 years ago
That'd be mine :-)

The old code happened to work because it wasn't made aware that it was reading 
beyond the end of the string (though it's a segv waiting to happen).

Either we'll need to beef up CalculateUTF8Length to deal with non-UTF8 strings 
better, or assert that UTF8toUCS2 barfs on non-UTF8 strings and have a |IsUTF8| 
function for people who need that functionality.
Assignee: scc → jaggernaut

Comment 2

17 years ago
I'd prefer the latter, personally, and we could add #ifdef DEBUG 
assert-on-!IsUTF8 code to NS_ConvertUTF8toUCS2::Init and tell people to fix 
the call-site.


17 years ago
Blocks: 105889

Comment 3

17 years ago
Created attachment 54849 [details] [diff] [review]
One possible (temp.) fix

Comment 4

17 years ago
Created attachment 54974 [details] [diff] [review]
Another attemp

Comment 5

17 years ago
The first patch was really for bug 105971. The second patch should also fix the
problem, though in a slightly different way.
Comment on attachment 54974 [details] [diff] [review]
Another attemp

r=dbaron, although as we discussed you should probably change the
NS_ERROR to say that this converter should not be used for data coming off
the network but rather only for data encoded into UTF-8 by Mozilla.
Attachment #54974 - Flags: review+

Comment 7

17 years ago
Updated NS_ERROR. Checked in, marking fixed.
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.