Closed Bug 49724 Opened 24 years ago Closed 24 years ago

reading past end of string in nsHTMLToTXTSinkStream::AddToLine

Categories

(MailNews Core :: Composition, defect, P3)

Product:
MailNews Core
Component:
Composition
Platform:
x86
Windows NT
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: warrensomebody, Assigned: akkzilla)

Details

(Whiteboard: FIX IN HAND) 

While sending a message...

NTDLL! 77f7629c()
nsDebug::Assertion(const char * 0x01ffca54 
??_C@_0BG@EIKL@?$HMCharAt?$HM?5out?9of?9range?$AA@, const char * 0x01ffca70 
??_C@_0BA@OBAB@aIndex?$DMLength?$CI?$CJ?$AA@, const char * 0x01ffca84 
??_C@_0CH@IHJC@?4?4?2?4?4?2dist?2include?2nsAReadableSt@, int 0x00000220) line 
253 + 13 bytes
basic_nsAReadableString<unsigned short>::CharAt(unsigned int 0x00000000) line 
544 + 42 bytes
basic_nsAReadableString<unsigned short>::operator[](unsigned int 0x00000000) 
line 555
nsHTMLToTXTSinkStream::AddToLine(const unsigned short * 0x0012f248, int 
0x00000001) line 1261 + 13 bytes
nsHTMLToTXTSinkStream::Write(const nsString & {" "}) line 1496
nsHTMLToTXTSinkStream::AddLeaf(nsHTMLToTXTSinkStream * const 0x06491ce0, const 
nsIParserNode & {...}) line 992 + 28 bytes
CNavDTD::AddLeaf(const nsIParserNode * 0x02693ad0) line 3507 + 22 bytes
CNavDTD::HandleDefaultStartToken(CToken * 0x058d1c20, nsHTMLTag 
eHTMLTag_newline, nsIParserNode * 0x02693ad0) line 1141 + 12 bytes
CNavDTD::HandleStartToken(CToken * 0x058d1c20) line 1562 + 22 bytes
CNavDTD::HandleToken(CNavDTD * const 0x064a92e0, CToken * 0x058d1c20, nsIParser 
* 0x06490720) line 770 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x064a92e0, nsIParser * 0x06490720, 
nsITokenizer * 0x064a91c0, nsITokenObserver * 0x00000000, nsIContentSink * 
0x06491ce0) line 499 + 20 bytes
nsParser::BuildModel() line 1983 + 34 bytes
nsParser::ResumeParse(int 0x00000000, int 0x00000000) line 1864 + 11 bytes
nsParser::Parse(const nsString & {"<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 
Final//EN">
<html><head></head><body>
 

<center><table border="1" cols="1" wid"}, void * 0x00000000, const nsString & 
{"text/html"}, int 0x00000000, int 0x00000001, nsDTDMode eDTDMode_autodetect) 
line 1674 + 15 bytes
ConvertBufToPlainText(nsString & {"<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 
Final//EN">
<html><head></head><body>
 

<center><table border="1" cols="1" wid"}, int 0x00000001) line 2008 + 41 bytes
nsMsgAttachmentHandler::UrlExit(unsigned int 0x00000000, const unsigned short * 
0x100a20a0 gCommonEmptyBuffer) line 917 + 28 bytes
FetcherURLDoneCallback(nsIURI * 0x06496e10, unsigned int 0x00000000, const char 
* 0x06490e90, const char * 0x00000000, int 0x000024b2, const unsigned short * 
0x100a20a0 gCommonEmptyBuffer, void * 0x06496c50) line 442 + 16 bytes
nsURLFetcher::OnStopRequest(nsURLFetcher * const 0x06497a70, nsIChannel * 
0x06490f70, nsISupports * 0x00000000, unsigned int 0x00000000, const unsigned 
short * 0x100a20a0 gCommonEmptyBuffer) line 322 + 54 bytes
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x06490db0, 
nsIChannel * 0x06490f70, nsISupports * 0x00000000, unsigned int 0x00000000, 
const unsigned short * 0x100a20a0 gCommonEmptyBuffer) line 269
nsFileChannel::OnStopRequest(nsFileChannel * const 0x06490f78, nsIChannel * 
0x06490cf0, nsISupports * 0x00000000, unsigned int 0x00000000, const unsigned 
short * 0x100a20a0 gCommonEmptyBuffer) line 647 + 45 bytes
nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x06490820) line 
302
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x064903b0) line 97 + 12 bytes
PL_HandleEvent(PLEvent * 0x064903b0) line 587 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f6a660) line 528 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00b807bc, unsigned int 0x0000c0b6, unsigned 
int 0x00000000, long 0x00f6a660) line 1043 + 9 bytes
USER32! 77e71820()
00f
Accepting and will take a look; adding Daniel, the expert on the AddToLine
wrapping code.
This is simple. I have a fix and will checkin if I get approval. Brendan? It's 
the normal assumption that the char after a string will be the null char which 
hasn't been correct for ages (months at least).

diff -u -r3.79 nsHTMLToTXTSinkStream.cpp
--- nsHTMLToTXTSinkStream.cpp   2000/07/28 23:18:09     3.79
+++ nsHTMLToTXTSinkStream.cpp   2000/08/21 21:29:17
@@ -1256,9 +1256,10 @@
         mCurrentLine.Truncate();
         // Space stuff new line?
         if(mFlags & nsIDocumentEncoder::OutputFormatFlowed) {
-          if((restOfLine[0] == '>') ||
-             (restOfLine[0] == ' ') ||
-             (!restOfLine.CompareWithConversion("From ",PR_FALSE,5))) {
+          if((restOfLine.Length()>0) &&
+             ((restOfLine[0] == '>') ||
+              (restOfLine[0] == ' ') ||
+              (!restOfLine.CompareWithConversion("From ",PR_FALSE,5)))) {
             // Space stuffing a la RFC 2646 (format=flowed).
             mCurrentLine.AppendWithConversion(' ');
           }
Whiteboard: FIX IN HAND
Thanks, Daniel!  r=akkana if you want it.  Or I could batch it into some other
changes I have pending to that file.
Status: NEW → ASSIGNED
a=brendan@mozilla.org on the spot-fix.

But I'm cc'ing scc, because it seems to me any string abstraction that purveys
NUL terminators in some circumstances should do it in all.  Are you saying that
nsString (or a subtype or related type) implements operator [] in such a way
that s[s.Length()] != 0, and in fact might reference uninitialized or even
unowned memory?  That sounds like a bug in the operator [] implementation.

/be
I really should leave this to Scott, but I have heard him explain the reason 
before so I give it a try. The assumption that strings end with a NULL char is 
only a C convention and it doesn't fit very well when trying to share buffers 
between string and other strings that are substrings of the first string for 
instance. 

Then again, you could patch the [] operator (and maybe others too) to return 
NULL for the char after the last char I guess. Oh well, I leave this to Scott. 
:-)

And Akkana, you can checkin this with your other changes. Easiest for us all.

Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
changing qa
QA Contact: lchiang → esther
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.