Csv import by seatedscribe · Pull Request #148 · keepassx/keepassx

seatedscribe · 2016-02-06T01:59:34Z

CSV IMPORT FEATURE

As promised, now it is possible for KeepassX to import CSV text files.

PARSER

I developed the parser with a BNF approach for syntax validation; just for reference, this is the RFC grammar as provided by the IETF (RFC 4180):

file = [header CRLF] record *(CRLF record) [EOF] #[CRLF]
header = name *(COMMA name)
record = field *(COMMA field)
name = field
field = (escaped | non-escaped)
escaped = DQUOTE *(TEXTDATA | COMMA | CR | LF | 2DQUOTE) DQUOTE
non-escaped = *TEXTDATA
COMMA = %x2C
CR = %x0D
DQUOTE = %x22
LF = %x0A
CRLF = CR LF
TEXTDATA = %x20-21 | %x23-2B | %x2D-7E

I needed to validate the two most used versions - the "doubling quotes" and
the "escape character" - into the same engine, so I slightly modified
the grammar to better reflect actual code implementation. I also extended it
to validate the case of "escaped characters". This is the result:

file = record *(CRLF record) EOF
record = field *(separator field) | comment --e.g. ,,A,B has 2 empty fields and is valid
field = simple | quoted
simple = TEXT -- e.g. f""ie"l"""d is OK
quoted = qualifier escaped qualifier
escaped = ESC_TEXT *(escape_mark ESC_TEXT)
escape_mark = (2qualifier | BACKSLASH qualifier)
qualifier = one of {DQUOTE, COLON, BACKSLASH}
separator = one of {COMMA, COLON, SEMICOLON}
comment = *(SPACE | TAB) COMMENT
COMMA = %x2C --i.e. ','
COLON = %x58 --i.e. ':'
SEMICOLON = %x59 --i.e. ';'
COMMENT = %x23 --i.e. '#'
DQUOTE = %x22 --i.e. '"'
BACKSLASH = %x5C --i.e. ''
CR = %x0D
LF = %x0A
SPACE = %x20
CRLF = CR LF | LF
TEXT = *(any char except CRLF, separator, EOF) (also TEXT="" shall be valid)
ESC_TEXT = +(any char except qualifier, EOF)

GUI

I enjoyed putting some effort in GUI design to make the user experience enjoyable and at the same time to be future-proof and avoid reinventing the wheel; for further details look at the comment around CsvImportWidget::m_columnheader.

TEST

Last but not least, in particular when we talk about security, TDD is a must. I think I put for the parser testing all the conceivable cases together and get to successfully pass them at the end.

Hope will help and be useful!

Regards,
Enrico

…v_import

imsdigital · 2016-02-06T09:03:36Z

Sweet.

Thank you, this is brilliant.

Steve

On 6 Feb 2016, at 01:59, Tarquin Winot [email protected] wrote:

CSV IMPORT FEATURE

As promised, now it is possible for KeepassX to import CSV text files.

PARSER

I developed the parser with a BNF approach for syntax validation; just for reference, this is the RFC grammar as provided by the IETF (RFC 4180):

file = [header CRLF] record *(CRLF record) [EOF] #[CRLF]
header = name *(COMMA name)
record = field *(COMMA field)
name = field
field = (escaped | non-escaped)
escaped = DQUOTE *(TEXTDATA | COMMA | CR | LF | 2DQUOTE) DQUOTE
non-escaped = *TEXTDATA
COMMA = %x2C
CR = %x0D
DQUOTE = %x22
LF = %x0A
CRLF = CR LF
TEXTDATA = %x20-21 | %x23-2B | %x2D-7E

I needed to validate the two most used versions - the "doubling quotes" and
the "escape character" - into the same engine, so I slightly modified
the grammar to better reflect actual code implementation. I also extended it
to validate the case of "escaped characters". This is the result:

file = record *(CRLF record) EOF
record = field *(separator field) | comment --e.g. ,,A,B has 2 empty fields and is valid
field = simple | quoted
simple = TEXT -- e.g. f""ie"l"""d is OK
quoted = qualifier escaped qualifier
escaped = ESC_TEXT *(escape_mark ESC_TEXT)
escape_mark = (2qualifier | BACKSLASH qualifier)
qualifier = one of {DQUOTE, COLON, BACKSLASH}
separator = one of {COMMA, COLON, SEMICOLON}
comment = *(SPACE | TAB) COMMENT
COMMA = %x2C --i.e. ','
COLON = %x58 --i.e. ':'
SEMICOLON = %x59 --i.e. ';'
COMMENT = %x23 --i.e. '#'
DQUOTE = %x22 --i.e. '"'
BACKSLASH = %x5C --i.e. ''
CR = %x0D
LF = %x0A
SPACE = %x20
CRLF = CR LF | LF
TEXT = *(any char except CRLF, separator, EOF) (also TEXT="" shall be valid)
ESC_TEXT = +(any char except qualifier, EOF)

GUI

I enjoyed putting some effort in GUI design to make the user experience enjoyable and at the same time to be future-proof and avoid reinventing the wheel; for further details look at the comment around CsvImportWidget::m_columnheader.

Hope will help!

Regards,
Enrico

You can view, comment on, or merge this pull request online at:

#148 #148
Commit Summary

CSV import feature delivered.
Merge branch 'master' of https://github.com/keepassx/keepassx into csv_import
File Changes

M src/CMakeLists.txt https://github.com/keepassx/keepassx/pull/148/files#diff-0 (5)
A src/core/CsvParser.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-1 (411)
A src/core/CsvParser.h https://github.com/keepassx/keepassx/pull/148/files#diff-2 (99)
M src/format/KeePass2Writer.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-3 (2)
M src/gui/ChangeMasterKeyWidget.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-4 (6)
M src/gui/ChangeMasterKeyWidget.h https://github.com/keepassx/keepassx/pull/148/files#diff-5 (3)
M src/gui/ChangeMasterKeyWidget.ui https://github.com/keepassx/keepassx/pull/148/files#diff-6 (12)
M src/gui/DatabaseTabWidget.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-7 (17)
M src/gui/DatabaseTabWidget.h https://github.com/keepassx/keepassx/pull/148/files#diff-8 (1)
M src/gui/DatabaseWidget.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-9 (28)
M src/gui/DatabaseWidget.h https://github.com/keepassx/keepassx/pull/148/files#diff-10 (4)
M src/gui/MainWindow.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-11 (4)
M src/gui/MainWindow.ui https://github.com/keepassx/keepassx/pull/148/files#diff-12 (21)
A src/gui/csvImport/CsvImportWidget.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-13 (250)
A src/gui/csvImport/CsvImportWidget.h https://github.com/keepassx/keepassx/pull/148/files#diff-14 (78)
A src/gui/csvImport/CsvImportWidget.ui https://github.com/keepassx/keepassx/pull/148/files#diff-15 (524)
A src/gui/csvImport/CsvImportWizard.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-16 (72)
A src/gui/csvImport/CsvImportWizard.h https://github.com/keepassx/keepassx/pull/148/files#diff-17 (55)
A src/gui/csvImport/CsvParserModel.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-18 (139)
A src/gui/csvImport/CsvParserModel.h https://github.com/keepassx/keepassx/pull/148/files#diff-19 (59)
M src/main.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-20 (2)
M tests/CMakeLists.txt https://github.com/keepassx/keepassx/pull/148/files#diff-21 (11)
A tests/TestCsvParser.cpp https://github.com/keepassx/keepassx/pull/148/files#diff-22 (421)
A tests/TestCsvParser.h https://github.com/keepassx/keepassx/pull/148/files#diff-23 (71)
Patch Links:

https://github.com/keepassx/keepassx/pull/148.patch https://github.com/keepassx/keepassx/pull/148.patch
https://github.com/keepassx/keepassx/pull/148.diff https://github.com/keepassx/keepassx/pull/148.diff
—
Reply to this email directly or view it on GitHub #148.

raedah · 2016-05-27T05:12:10Z

I wanted to test this, but I couldnt find any of the required build deps for keepassx on arch. If there is some other way for me to test it, let me know.

seatedscribe · 2016-05-30T10:28:35Z

Hello raedah,

Could you be more specific about your issue? The import feature requires no
additional dependencies than KeepassX itself.
I don't own an archLinux workstation to try, but I would say it's the usual
process of building the project...

Enrico
On 27 May 2016 07:12, "raedah" [email protected] wrote:

I wanted to test this, but I couldnt find any of the required build deps
for keepassx on arch. If there is some other way for me to test it, let me
know.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#148 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AMqpZMW083Ya7LwcT0zP7eiX1nVKKDnuks5qFn03gaJpZM4HUwyk
.

victorhaggqvist · 2016-05-30T15:04:11Z

@raedah You may pickup some pointers from https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=keepassx-git

dzhus · 2016-06-02T11:13:39Z

Sweet! Works like charm.

For some reason building it fails with

/Users/dmitrydzhus/projects/keepassx/src/gui/csvImport/CsvImportWidget.cpp:225:5: error: invalid operands to binary expression ('bool' and 'QString')
    Q_ASSERT(label);
    ^~~~~~~~~~~~~~~

I changed the line to Q_ASSERT(!label.isNull()); and it fixed the compilation. I have Qt5 5.6.0 installed.

dakra · 2016-11-29T03:16:46Z

I like this as well.
Is there a chance you could rebase it and make a pull request at keepassxc

Unlike here, they actually comment on and merge pull requests ;)

seatedscribe · 2016-11-30T16:00:49Z

@dakra hehehe you read my mind... I was already on the task and I just made the pull request :)

CSV import feature delivered.

3318f05

seatedscribe force-pushed the csv_import branch 4 times, most recently from a3712bc to fa9375e Compare February 6, 2016 02:52

Merge branch 'master' of https://github.com/keepassx/keepassx into cs…

036a1ea

…v_import

seatedscribe force-pushed the csv_import branch 2 times, most recently from c89c10b to 036a1ea Compare February 6, 2016 02:59

seatedscribe mentioned this pull request Dec 1, 2016

CSV import feature keepassxreboot/keepassxc#120

Closed

This was referenced Jan 9, 2017

Implement import of databases in CSV (Comma Separated Values) format keepassxreboot/keepassxc#145

Closed

Implement import of databases in CSV (Comma Separated Values) format keepassxreboot/keepassxc#146

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Csv import#148

Csv import#148
seatedscribe wants to merge 2 commits intokeepassx:masterfrom
seatedscribe:csv_import

seatedscribe commented Feb 6, 2016

Uh oh!

imsdigital commented Feb 6, 2016

Uh oh!

raedah commented May 27, 2016

Uh oh!

seatedscribe commented May 30, 2016

Uh oh!

victorhaggqvist commented May 30, 2016

Uh oh!

dzhus commented Jun 2, 2016

Uh oh!

dakra commented Nov 29, 2016

Uh oh!

seatedscribe commented Nov 30, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

seatedscribe commented Feb 6, 2016

CSV IMPORT FEATURE

PARSER

GUI

TEST

Uh oh!

imsdigital commented Feb 6, 2016

Uh oh!

raedah commented May 27, 2016

Uh oh!

seatedscribe commented May 30, 2016

Uh oh!

victorhaggqvist commented May 30, 2016

Uh oh!

dzhus commented Jun 2, 2016

Uh oh!

dakra commented Nov 29, 2016

Uh oh!

seatedscribe commented Nov 30, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants