ASCIIPropertyListParser: allow escaping chars that needn't be escaped

[matvore]

No description provided.

[3breadt]

At the moment invalid escape sequences would lead to a failure in parsing the file. This is acceptable I believe.

With your commit they would be treated as if they were correct. Furthermore your code even assumes they have a special meaning, that the following character is just needlessly escaped. But there is no way to be sure that, for example, "\d" should be interpreted as "d" and not as "$". The creator of the property list may have defined his own custom escape sequences, only supported by his own parser. Or he may have simply made a mistake.

Whatever the reason for the invalid escape sequence, it remains invalid. Thus, I won't accept your pull request.

[matvore]

Are you aware that "defaults" interprets the characters as my pull request does (this was in the code, I'm not sure if you caught it):

$ printf '"a" = "\\c";\n' > /tmp/foo.plist
$ defaults read /tmp/foo.plist
{
    a = c;
}

[3breadt]

No I was not aware of it. But after fiddling with it a bit, I see it does not really escape all invalid characters consistently. Also XCode's property list viewer handles the invalid escape sequence even worse. I really would not like to take either program's behavior as a pattern for this library.

[matvore]

\a is an alert bell
\f is formfeed
\v is vertical tab

(see https://en.wikipedia.org/wiki/Escape_sequences_in_C)

These characters are apparently not supported in ASCII plists. Remove them from your sample string and you'll get consistent output between defaults and Xcode. So I see two options:

• keep pull request as-is but issue an error for \a \f or \v
• change pull request to only work for ' or " and all non-necessary escape characters cause an exception like they used to (supporting quotes was the original intent of the change, but I didn't realize \a \f and \v were exceptional at the time I wrote it)

Which do you prefer? I'm happy with either one.

[3breadt]

I already improved the exception text thrown when an invalid escape sequence is encountered in b8d6987.

The only actual reference describing the ASCII property list format I found at http://www.gnustep.org/resources/documentation/Developer/Base/Reference/NSPropertyList.html
If I go by that, single quotes would not be allowed.

In reality tools like defaultsmay implement the complete set of (Objective-)C escape sequences. I would be ok with also adding support for that well-defined set of escape sequences. This would mean adding support for:

• \a
• \f
• \v
• \'
• \?
• \x...

Would that be fine with you?

[matvore]

Not sure what \? is. I would just throw an exception on \a \f and \v, since they appeared mangled in Xcode and defaults interpreted them as \\a. Other than that, the NSPropertyList.html document and your list are fine.

I'm mostly concerned about the single quote. We had tooling which escaped these single quotes (maybe it was an old version of Xcode, or maybe internal tooling) when it wrote Localizable.strings files - the Xcode build process is fine with these.

[3breadt]

Alright, now I've just added a switch case to allow for the single quote (See 44ecdc2). I will leave out the other C escape sequences for the moment as nobody else was bothered by the missing support so far.

[matvore]

Great! Thank you.

[matvore]

I changed this PR's commit to simply add a test for escaping the single quote. Can you merge it in the current form?

[3breadt]

Done