keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
21.48k stars 1.48k forks source link

Text-/Comment-Qualifiers corrupt csv import data #11502

Open q6276270 opened 4 days ago

q6276270 commented 4 days ago

Overview

In KeypassXC 2.6.6 importing a CSV file a text qualifiers contained in a field breaks the line. Also the recognition of text qualifiers is greedy within the current csv-entry, i.e. it doesn't check wether it is a textqualifier directly followed by a separator or linebreak to close the field.

Steps to Reproduce

  1. Try import csv
  2. Try importing a file with fields containing 3 textqualifiers among other random strings or ending a csv field early with a closing textqualfier.

Expected Behavior

I'd have thought a textqualifier qualifies the entire csv-field as text. To parse this one would have to check for the opening qualifier directly after a linebreak or previous separator and the closing qualfier directly followed by either a linebreak or a separator. Alternatively it should be possible to disable text/comment-qualifiers.

Actual Behavior

The early closing of the text-qualifier in a csv-field acts similar to a linebreak.

KeePassXC - 2.6.6 Revision: 9c108b9

Operating System: Linux Desktop Env: Cinnamon

droidmonkey commented 4 days ago

Update to 2.7.9

What you describe, however, sounds like "invalid" CSV formatting. CSV is not a well defined standard, but there are some norms. We support the norms and they are well tested.

Also, you should provide a sample CSV file that fails your test if you want a second opinion.

q6276270 commented 4 days ago

In 2.7.9 it also fails with https://pastebin.com/vMT8DEum, separator as "\t" and text qualifier as "|". It probably is due to invalid CSV-Formatting if you assume a CSV-Format with parsing of qualifiers. That's why I suggested an option to disable qualifiers. Surely a format without them would be valid? It's impossible to automatically and correctly import such a file at the moment if data in the fields contain all of the available qualifiers at some point.

droidmonkey commented 4 days ago

Ok i can replicate the behavior and it needs fixing, thank you.