47476 - nsCID::Parse() accepts invalid CID strings.

Status: NEW

(Core :: XPCOM, defect, P5)

Description

[Judson Valeski]

If you had Parse() a bogus, inproperly formated string, it will still succeed.

Comment 1

[Suresh Duddi (gone)]

Ray ?

Comment 2

[Ray Whitmer]

I have been looking through source code trying with no success to find an 
example of where this call is used.

As described, this seems to be a low-priority bug.  It only causes a problem if 
someone passes in an improperly-formatted string.  They will probably fail as 
soon as they try to use the CID.  The code in nsID.cpp of the Parse method is 
checking the length and nothing else to compute the return value.  This really 
should be a programming error, not a run-time error, it seems to me.  If this 
were often-used, it would seem silly for this method to be called passing in a 
constant, yet checking the return, since the constant never changes.  If it is 
silly to check the return, then it seems silly to even provide the return, 
unless there is some case where these values are being suplied not as constants.

What are some significant use cases that this bug fixes?

Comment 3

[Judson Valeski]

I fixed a place that was using it to no longer use it because it's broken.

Comment 4

[Ray Whitmer]

Can you tell me a bit more about the use case involved?  Was it a literal string 
it was trying to parse, or did the string come from some other source?

Also, the bug report mentioned breakage only when the syntax was wrong.  Did it 
not function correctly when the syntax was right, or is wrong syntax a normal 
part of the operation of this particular use case?

Comment 5

[Judson Valeski]

wrong syntax was being passed in, the use case was busted. this bug is just to 
tell the user when parse() is doing something it doesn't handle well.

here's what I ripped out.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&fi
le=nsHTTPHandler.cpp&root=/cvsroot&subdir=mozilla/netwerk/protocol/http/src&comm
and=DIFF_FRAMESET&rev1=1.123&rev2=1.124

Comment 6

[Ray Whitmer]

I agree that the use case is busted, based upon the diff.

Thanks, I was just trying to discover any use cases for the method that might 
require it to be fixed nsbeta3.

Comment 7

[leger]

Adding verifyme keyword.

Comment 8

[Warren Harris]

Edward: Welcome to xpcom!

Comment 9

[Warren Harris]

Once again... attempting to reassign from Ray to Edward.

Comment 10

[Doug Turner (:dougt)]

reassign all kandrot xpcom bug.