Quotes in attribute name or value make the PCB file unreadable.

[Milan Prochac]

When attributes are written into PCB file, strings are not sanitized. Quotes or backslashes in attribute name or attribute value lead to syntactically incorrect PCB file.

Solution: use existing PrintQuotedString function to sanitize attribute value and attribute name when PCB file is saved.

Fix is provided in attached patch.

[Bert Timmerman (bert-timmerman)]

Hi,

I still need to triage this in more detail, however I consider breaking a pcb file with a couple of double quotes a bug of high importance.

I created a topic branch "LP1506204" in the pcb git repository.

Thanks for the patch.

[Bert Timmerman (bert-timmerman)]

Hi,

Thinking some more on this subject I think efforts should be made to:

1) prevent the insertion of quotes in attribute names or attribute values from the pcb UIs ,

2) make the file parser more robust in case a hand edited pcb file contains quotes in attribute names or attribute values.

[Milan Prochac (milan-x)]

I agree that quotes can be prevented in attribute names, but they should be allowed in the attribute values - in my specific case I use attributes in my export filter for post-processing and sometimes I need the quotes in the string...

Preventing the insertion of quotes and backslashes requires requires non-trivial (more than 5 lines patch) changes in the UI: will be user notified or forbidden characters will be silently ignored when typed? What kind of notification? Dialog window? Color change?

Quotes are allowed in other texts - like reference designator, where are quite unusual - so allowing them in attributes will keep consistent text behaviour across whole application.

[Bert Timmerman (bert-timmerman)]

Maybe I misunderstood:

How does sanitizing quotes in attribute name and attribute value when writing to file align with allowing them during text entry (input from the user), or during parsing of the newly loaded pcb file.

IMHO, quotes are either banned or allowed, and this should be consistent in all three use cases.

[Milan Prochac (milan-x)]

Maybe the word "sanitization" leads to confusion - it does not mean to remove the "forbidden" characters, it just means to escape them to make strings safe.

Currently the quotes in the strings are allowed in all strings in PCB and they are usually escaped when written in the PCB file (and unescaped when read from file). For example when I enter R"1" as reference designator it is saved as "R\"1\"" in the PCB file and it is understood correctly when the PCB file is read.

But this is not true for attributes - they are stored "as is", no escaping is done - so value R"1" is saved as "R"1"" and such file fails to load afterwards.

The patch provided adds the missing escaping and IMO solves the the issue completely:
- saved file is syntactically correct
- contents of PCB layout is unchanged, attribute values and names are preserved (including quotes and backslashes)
- behaviour is consistent with other text entries in PCB

I was just opposing the necessity of UI changes to prevent to enter the quotes...

[Bert Timmerman (bert-timmerman)]

Yes, just what I thought: I misunderstood some aspects.

So it's just one of the three cases where things go wrong, unless one "intentionally" breaks the file format with an editor and unescaping quotes in attribute [name,value] strings.

I will review the patch.

[Bert Timmerman (bert-timmerman)]

Pushed to master