281977 - nsStringInputStream botches -1 lengths

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[timeless]

>	xpcom_core.dll!nsStringInputStream::ReadSegments(unsigned int 
(nsIInputStream *, void *, const char *, unsigned int, unsigned int, unsigned 
int *)* writer=0x013b57d1, void * closure=0x0012f428, unsigned int 
aCount=4294967295, unsigned int * result=0x0012f4a0)  Line 251	C++
 	docshell.dll!nsDocShell::AddHeadersToChannel(nsIInputStream * 
aHeadersData=0x00000000, nsIChannel * aGenericChannel=0x002a0178)  Line 5616
	C++
 	docshell.dll!nsDocShell::DoURILoad(nsIURI * aURI=0x02780520, nsIURI * 
aReferrerURI=0x00000000, nsISupports * aOwner=0x024d69c8, const char * 
aTypeHint=0x0283111c, nsIInputStream * aPostData=0x00000000, nsIInputStream * 
aHeadersData=0x02836ee8, int firstParty=0, nsIDocShell * * 
aDocShell=0x00000000, nsIRequest * * aRequest=0x00000000)  Line 5542 + 0xf
	C++
 	docshell.dll!nsDocShell::InternalLoad(nsIURI * aURI=0x00888020, nsIURI 
* aReferrer=0x0083ec03, nsISupports * aOwner=0x02832000, int aInheritOwner=0, 
const unsigned short * aWindowTarget=0x02831380, const char * 
aTypeHint=0x002a0168, nsIInputStream * aPostData=0x00000000, nsIInputStream * 
aHeadersData=0x00000000, unsigned int aLoadType=42144640, nsISHEntry * 
aSHEntry=0x00000028, int firstParty=42144648, nsIDocShell * * 
aDocShell=0x00000000, nsIRequest * * aRequest=0x002a0178)  Line 5356 + 0x25
	C++
 	docshell.dll!nsDocShell::LoadURI(nsIURI * aURI=0x02780520, 
nsIDocShellLoadInfo * aLoadInfo=0x00000000, unsigned int aLoadFlags=1, int 
firstParty=1)  Line 757 + 0x31	C++
 	docshell.dll!nsDocShell::LoadURI(const unsigned short * 
aURI=0x02831380, unsigned int aLoadFlags=40, nsIURI * aReferringURI=0x02831388, 
nsIInputStream * aPostStream=0x00000000, nsIInputStream * 
aHeaderStream=0x002a0178)  Line 2566	C++
 	xpcom_core.dll!XPTC_InvokeByIndex(nsISupports * that=0x024fd158, 
unsigned int methodIndex=8, unsigned int paramCount=5, nsXPTCVariant * 
params=0x0012f7b8)  Line 102	C++
 	xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, 
XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2034 + 0x16	C++

code uses a stringinputstream as a param to loaduri and addheaderstochannel 
passes -1 and the code in nsStringInputStream::ReadSegments gets signed math 
wrong while doing comparisons.

	*result	4294967295	unsigned int
	mOffset	-1	int
	mLength	463	int

we're using mozilla1.8a5 but the code is still buggy on trunk :)

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

hmm, would this be fixed by making
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsStringStream.cpp#244:
244     if ((PRInt32)aCount > maxCount)

use an unsigned comparison?

Comment 2

[Darin Fisher]

I think this bug is important to fix for 1.8 beta 2.  If someone has cycles,
please help out with a patch.  I'll do my best to get to it otherwise.

Comment 3

[Darin Fisher]

> use an unsigned comparison?

yes, that is precisely the fix i recommended to timeless over irc.

Comment 4

[timeless]

Attachment: use unsigned ints

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 175184 [details] [diff] [review]
use unsigned ints

+	 NS_ASSERTION(*result <= aCount, "writer should tell us how much it
wrote");

I'd use something more like "writer returned too large length"...

Comment 6

[Darin Fisher]

Comment on attachment 175184 [details] [diff] [review]
use unsigned ints

>Index: nsStringStream.cpp
...
>+    if (aCount > maxCount)
>         aCount = maxCount;
>+    NS_ASSERTION(aCount != PR_UINT32_MAX, " stream should never pass PR_UINT32_MAX as the count parameter to the writer");

so, i'm not sure what this assertion does for us given that
we've limited aCount to be no more than maxCount.  I don't
think that maxCount will ever be PR_UINT32_MAX, so this is
a pretty bogus assertion, no?

sr=darin

Comment 7

[timeless]

Comment on attachment 175184 [details] [diff] [review]
use unsigned ints

mozilla/xpcom/io/nsStringStream.cpp	1.18