PCBNew: Quotes in Text objects cause text to get truncated

Since this is all pretty fresh in my old head, having just solved the issues
like this for the s-expression support, I stepped up and committed a partial
solution for the old board format too. The strategy used can be applied in
many places, to fix this in a general and comprehensive way.

The strategy involves "escaping" of double quotes with a preceding \
character, and doubling up the \ byte when present alone. (The s-expression
support went further, and that was to translate CR NL bytes to \r and \r two
byte pairs, but that was not done here.)

1) ReadDelimitedText() was revised to handle the escaping.

2) std::string EscapedUTF8( const wxString& aString ) was written to
complement the revised ReadDelimitedText() function for writing out these
strings. See line 166 of class_pcb_text.cpp as an example of what needs to
be done everywhere that ReadDelimitedText() is used as the inverse for reading.

More work needs to be done planting EscapedUTF8() in many places. The
reported bug's usage of " was only one of many potential problems handling
double quotes, but they can all be fixed simply by using EscapedUTF8().

There is a minor vulnerability in making the transition, if you have a \
byte in your board file already, since it is not currently doubled up, when
we read it we will basically skip it now, although I suppose a minor
enhancement would be to only skip it if the next byte is either " or \.

Dick

On 02/26/2011 11:54 PM, Dick Hollenbeck wrote:
> There is a minor vulnerability in making the transition, if you have a \
> byte in your board file already, since it is not currently doubled up, when
> we read it we will basically skip it now, although I suppose a minor
> enhancement would be to only skip it if the next byte is either " or \.

Made that enhancement.

And I think this means it is safe to immediately use the new code everywhere
in the current/old data files.

Dick

When expanding usage of EscapedUTF8(), some issues to keep in mind are:

1) Can the reading code be made string "line offset/position" independent, now that escape sequences can expand the length of a field. ReadDelimitedText() returns the number of bytes consumed, so of course yes, but it needs to be looked at now more closely, rather than just assuming that a certain field will be at a certain fixed offset. Instead, an accumulation of the return values from ReadDelimitedText() have to be used to establish the next field's offset.

2) EscapedUTF8() could easily be changed to add the wrapping double quotes and thereby simply the fprintf() format string that is used everywhere, by makeing \"%s\" be simply %s. This is a pending design decision, and is a reason why I did not install EscapedUTF8() everywhere just yet.

Opinions welcome. I am done working on this for now. Sort of feel like anything we do is only temporaty anyway, pending an eventual migration to s-expressions, where I feel the issue of embedded quotes and control characters has been solved is a solid way with OUTPUTFORMATTER::Quoted() and DSNLEXER::NextTok().

r2851 stable flagged this as fixed.