Closed Bug 28197 Opened 25 years ago Closed 25 years ago

NS_NewURI damange non ASCII data

Categories

(Core :: Networking, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: ftang, Assigned: warrensomebody)

Details

Attachments

(1 file)

Currently, NS_NewURI have the nsString favor of interface but it damage the 
non-ASCII data when it call ToCString.


 53             inline nsresult
 54             NS_NewURI(nsIURI* *result, const nsString& spec, nsIURI* baseURI 
= nsnull)
 55             {
 56                 // XXX if the string is unicode, GetBuffer() returns null. 
 57                 // XXX we need a strategy to deal w/ unicode specs (if there 
should
 58                 // XXX even be such a thing)
 59                 char* specStr = spec.ToNewCString(); // this forces a single 
byte char*
 60                 if (specStr == nsnull)
 61 warren  1.1         return NS_ERROR_OUT_OF_MEMORY;
 62                 nsresult rv = NS_NewURI(result, specStr, baseURI);
 63                 nsAllocator::Free(specStr);
 64                 return rv;
 65             }

Suggestion- Either remove the nsString favor to push the responsability out from 
Necko or call ToNewUTF8String to reserved Unicode data and let the protcol 
handler deal with it in individual protocol.

I don't think this is the *final* soultion for bug 14155 and 28171. But current 
call to ToCString is definitely wrong. Change it to call ToNewUTF8String may not 
solve the whole issue, but it have no possiblility to make it worst.
Blocks: 14155
No longer blocks: 14155
I vote for fixing NS_NewURI.

The call to ToNewCstring corrupts any non-ASCII data.
Changing the call ToNewUTF8String will not change data handling for the ASCII
case and will preserve the non-ASCII data.

Frank, Isn't this just a 1-line change?
Target Milestone: M15
There are 2 evil places in the nsNetUtil.h header that do that.
This is a one liner and it is blocking QA finding more i18n problem. 
Keywords: beta1
change to ToNewUTF8String and check in
Status: NEW → ASSIGNED
Can I get a review on this please?
whoops. we crossed paths. I'm marking this fixed. I also pulled the dead comment
out above one of the ToNewCString() calls.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
verified:  NT 2000042009
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: