Page MenuHomePhabricator

Sanitize UTF8 more aggressively to satisfy json_encode()
ClosedPublic

Authored by epriestley on Aug 24 2016, 3:53 PM.
Tags
None
Referenced Files
F13281294: D16440.diff
Sun, Jun 2, 10:31 AM
F13268506: D16440.diff
Wed, May 29, 5:07 AM
F13252101: D16440.diff
Sat, May 25, 1:04 AM
F13223926: D16440.id39537.diff
Sun, May 19, 5:22 AM
F13204381: D16440.diff
Wed, May 15, 12:31 AM
F13187305: D16440.diff
Sat, May 11, 4:27 AM
F13176637: D16440.id.diff
Wed, May 8, 3:48 PM
F13176136: D16440.diff
Wed, May 8, 10:52 AM
Subscribers
None

Details

Summary

Fixes T11525. Currently, there are some strings such that:

json_encode(phutil_utf8ize($string));

...fails. I encountered this with DarkConsole trying to JSON encode queries that inserted encrypted file data into the MySQL blob store, so basically random data.

There appear to be two cases we aren't handling well:

  • Overlong representations: Shorter characters can be written in an invalid way with more bytes. We previously allowed these -- sometimes -- but json_encode() does not. Instead, reject them. We already rejected overlong 2-character codes.
  • Surrogate characters: There is a range of surrogate characters reserved for use in UTF16 which json_encode() rejects. Just reject these ourselves, too.
Test Plan

Wrote a bunch of test cases to cover this stuff, all of which now pass.

Fuzzed json_encode(phutil_utf8ize($string)) on random strings in a loop. Before these changes it would fail after a handful of attempts, in less than a second. After these changes, I ran it for several minutes and didn't see any failures.

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Sanitize UTF8 more aggressively to satisfy json_encode().
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.
chad added inline comments.
src/utils/utf8.php
446–461

¯\_(ツ)_/¯

This revision is now accepted and ready to land.Aug 24 2016, 4:12 PM
This revision was automatically updated to reflect the committed changes.
src/utils/utf8.php
446–461

look how nicely formatted it is

it must be right!