[PDF] Fix name generation - / needs to be escaped.

[PDF] Fix name generation - / needs to be escaped.

BUG=chromium 148422

Committed: https://code.google.com/p/skia/source/detail?r=5641

[edisonn, 2012-09-20 14:35:40]

LGTM - see comments

https://codereview.appspot.com/6542044/diff/1/src/pdf/SkPDFTypes.cpp
File src/pdf/SkPDFTypes.cpp (right):

https://codereview.appspot.com/6542044/diff/1/src/pdf/SkPDFTypes.cpp#newcode308
src/pdf/SkPDFTypes.cpp:308: input[i] == '/') {
Can you make a test which will get all characters from \0 to \255 and make sure
there is nothing else which needs encoding? A manual check should be enough.

[Steve VanDeBogart, 2012-09-20 17:23:55]

https://codereview.appspot.com/6542044/diff/1/src/pdf/SkPDFTypes.cpp
File src/pdf/SkPDFTypes.cpp (right):

https://codereview.appspot.com/6542044/diff/1/src/pdf/SkPDFTypes.cpp#newcode308
src/pdf/SkPDFTypes.cpp:308: input[i] == '/') {
On 2012/09/20 14:35:41, edisonn wrote:
> Can you make a test which will get all characters from \0 to \255 and make
sure
> there is nothing else which needs encoding? A manual check should be enough.

So the "Name" section only mentions escaping whitespace, so that what I did.  I
thought that / needing escaping was an implementation bug.  But when looking for
what the spec says I found another section that says a bunch of characters
always start a new block (and thus implicitly have to be escaped in names).
Updated and added test that they are all escaped.

[edisonn, 2012-09-21 17:35:59]

https://codereview.appspot.com/6542044/diff/1002/src/pdf/SkPDFTypes.cpp
File src/pdf/SkPDFTypes.cpp (right):

https://codereview.appspot.com/6542044/diff/1002/src/pdf/SkPDFTypes.cpp#newco...
src/pdf/SkPDFTypes.cpp:308: if (input[i] & 0x80 || input[i] < '!' ||
strchr(escaped, input[i])) {
escaped string is small, so lgtm, but please add a comment that if the number of
chars needing escaping gets larger than, say 20, then we should use a form of
map instead

[edisonn, 2012-09-21 17:36:43]

LGTM

[Steve VanDeBogart, 2012-09-21 17:45:38]

https://codereview.appspot.com/6542044/diff/1002/src/pdf/SkPDFTypes.cpp
File src/pdf/SkPDFTypes.cpp (right):

https://codereview.appspot.com/6542044/diff/1002/src/pdf/SkPDFTypes.cpp#newco...
src/pdf/SkPDFTypes.cpp:308: if (input[i] & 0x80 || input[i] < '!' ||
strchr(escaped, input[i])) {
On 2012/09/21 17:35:59, edisonn wrote:
> escaped string is small, so lgtm, but please add a comment that if the number
of
> chars needing escaping gets larger than, say 20, then we should use a form of
> map instead

Done.