If we can't read a unicode character, write the standard "unknown" (0xFFFD) c...

If we can't read a unicode character, write the standard "unknown" (0xFFFD) character. This will prevent security issues where the current behaviour can be used to strip characters out of a string after it has passed some validation.

BUG=30798
TEST=utf_string_conversions_unittest.cc,utf_offset_string_conversions_unittest.cc,zip_unittest.cc


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=35430

[Chris Evans, 2009-12-31 09:42:33]

Hang fire on the review whilst I resolve a trybot unit test failure...

[jschuh, 2009-12-31 15:58:34]

On 2009/12/31 09:42:33, Chris Evans wrote:
> Hang fire on the review whilst I resolve a trybot unit test failure...

Oddly enough, you're breaking the unit test added with the fix for
http://code.google.com/p/chromium/issues/detail?id=27994

Want me to take a look at it since it's a Windows specific test?

[cevans, 2009-12-31 20:59:52]

On Thu, Dec 31, 2009 at 7:58 AM, <jschuh@chromium.org> wrote:

> On 2009/12/31 09:42:33, Chris Evans wrote:
>
>> Hang fire on the review whilst I resolve a trybot unit test failure...
>>
>
> Oddly enough, you're breaking the unit test added with the fix for
> http://code.google.com/p/chromium/issues/detail?id=27994


That's expected, because .<invalid>. used to get mapped to .. (which should
cause failure, and this is what you are testing for).
Now, it gets mapped to .U+FFFD. which is a valid path component I suppose.

What's surprising is that the test on Linux / Mac ever passed, since we
don't do character stripping conversions on those platforms. Looks like the
invalid byte sequence is causing EILSEQ error at some API. Investigating...


>
> Want me to take a look at it since it's a Windows specific test?
>
>
> http://codereview.chromium.org/522029
>

[Chris Evans, 2009-12-31 23:23:23]

Ok, Linux situation resolved. There was a spurious UTF8 -> Unicode -> UTF8
conversion on the filename. Fix up zip_unittest a bit too.

Ready for review. Fingers crossed on the try bot run :)

On 2009/12/31 20:59:52, cevans_google.com wrote:
> On Thu, Dec 31, 2009 at 7:58 AM, <mailto:jschuh@chromium.org> wrote:
> 
> > On 2009/12/31 09:42:33, Chris Evans wrote:
> >
> >> Hang fire on the review whilst I resolve a trybot unit test failure...
> >>
> >
> > Oddly enough, you're breaking the unit test added with the fix for
> > http://code.google.com/p/chromium/issues/detail?id=27994
> 
> 
> That's expected, because .<invalid>. used to get mapped to .. (which should
> cause failure, and this is what you are testing for).
> Now, it gets mapped to .U+FFFD. which is a valid path component I suppose.
> 
> What's surprising is that the test on Linux / Mac ever passed, since we
> don't do character stripping conversions on those platforms. Looks like the
> invalid byte sequence is causing EILSEQ error at some API. Investigating...
> 
> 
> >
> > Want me to take a look at it since it's a Windows specific test?
> >
> >
> > http://codereview.chromium.org/522029
> >
>

[jschuh, 2010-01-01 00:34:24]

LGTM