Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2025-03-09 15:49:22 -04:00
Code Issues Packages Projects Releases Wiki Activity

Fix various quirks with CSV import widget and parser

Browse Source 
* Fixes #11502 - correct improper handling of text qualifiers

* Improve layout of csv import widget
* Hide error messages when trying to import again
This commit is contained in:
Jonathan White 2024-11-29 14:41:43 -05:00
parent 244ed42231
commit 1b1643b5d1
8 changed files with 248 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
share/translations/keepassxc_en.ts
View File

@ -8682,18 +8682,6 @@ Kernel: %3 %4</source>
<source>file empty</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>malformed string</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>missing closing quote</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>%1: (row, col) %2,%3</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>AES 256-bit</source>
<translation type="unfinished"></translation>
@ -9232,6 +9220,18 @@ This option is deprecated, use --set-key-file instead.</source>
<source>start minimized to the system tray</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>malformed string, possible unescaped delimiter</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>missing closing delimiter</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>%1, row: %2, column: %3</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>QtIOCompressor</name>

21
src/format/CsvParser.cpp
View File

@ -1,4 +1,4 @@
/*
/*
* Copyright (C) 2016 Enrico Mariotti <enricomariotti@yahoo.it>
* Copyright (C) 2017 KeePassXC Team <team@keepassxc.org>
*
@ -53,7 +53,7 @@ bool CsvParser::reparse()
return parseFile();
}
bool CsvParser::parse(QFile* device)
bool CsvParser::parse(QIODevice* device)
{
clear();
if (!device) {
@ -66,7 +66,7 @@ bool CsvParser::parse(QFile* device)
return parseFile();
}
bool CsvParser::readFile(QFile* device)
bool CsvParser::readFile(QIODevice* device)
{
if (device->isOpen()) {
device->close();
@ -79,6 +79,7 @@ bool CsvParser::readFile(QFile* device)
} else {
device->close();
// Normalize on newline endings
m_array.replace("\r\n", "\n");
m_array.replace("\r", "\n");
if (m_array.isEmpty()) {
@ -121,7 +122,7 @@ bool CsvParser::parseFile()
parseRecord();
while (!m_isEof) {
if (!skipEndline()) {
appendStatusMsg(QObject::tr("malformed string"), true);
appendStatusMsg(QObject::tr("malformed string, possible unescaped delimiter"), true);
}
m_currRow++;
m_currCol = 1;
@ -161,7 +162,7 @@ void CsvParser::parseField(CsvRow& row)
{
QString field;
peek(m_ch);
if (m_ch != m_separator && m_ch != '\n' && m_ch != '\r') {
if (m_ch != m_separator && m_ch != '\n') {
if (isQualifier(m_ch)) {
parseQuoted(field);
} else {
@ -190,7 +191,7 @@ void CsvParser::parseQuoted(QString& s)
getChar(m_ch);
parseEscaped(s);
if (!isQualifier(m_ch)) {
appendStatusMsg(QObject::tr("missing closing quote"), true);
appendStatusMsg(QObject::tr("missing closing delimiter"), true);
}
}
@ -391,6 +392,12 @@ int CsvParser::getCsvRows() const
void CsvParser::appendStatusMsg(const QString& s, bool isCritical)
{
m_statusMsg += QObject::tr("%1: (row, col) %2,%3").arg(s, m_currRow, m_currCol).append("\n");
if (!m_statusMsg.isEmpty()) {
m_statusMsg.append("\n");
}
m_statusMsg +=
QObject::tr("%1, row: %2, column: %3").arg(s, QString::number(m_currRow), QString::number(m_currCol));
m_isGood = !isCritical;
}

6
src/format/CsvParser.h
View File

@ -22,7 +22,7 @@
#include <QBuffer>
#include <QTextStream>
class QFile;
class QIODevice;
typedef QStringList CsvRow;
typedef QList<CsvRow> CsvTable;
@ -34,7 +34,7 @@ public:
CsvParser();
~CsvParser();
// read data from device and parse it
bool parse(QFile* device);
bool parse(QIODevice* device);
bool isFileLoaded();
// reparse the same buffer (device is not opened again)
bool reparse();
@ -85,7 +85,7 @@ private:
void parseQuoted(QString& s);
void parseEscaped(QString& s);
void parseEscapedText(QString& s);
bool readFile(QFile* device);
bool readFile(QIODevice* device);
void reset();
void clear();
bool skipEndline();

29
src/gui/csvImport/CsvImportWidget.cpp
View File

@ -17,6 +17,7 @@
*/
#include "CsvImportWidget.h"
#include "ui_CsvImportWidget.h"
#include "core/Clock.h"
@ -145,6 +146,13 @@ void CsvImportWidget::updatePreview()
m_ui->spinBoxSkip->setRange(minSkip, qMax(minSkip, m_parserModel->rowCount() - 1));
m_ui->spinBoxSkip->setValue(minSkip);
// Store the previous column information for comparison later
auto prevColumns = m_comboModel->stringList();
QList<int> prevComboIndexes;
for (auto combo : m_combos) {
prevComboIndexes << combo->currentIndex();
}
QStringList csvColumns(tr("Not Present"));
auto parser = m_parserModel->parser();
for (int i = 0; i < parser->getCsvCols(); ++i) {
@ -159,6 +167,8 @@ void CsvImportWidget::updatePreview()
csvColumns << QString(tr("Column %1").arg(i));
}
}
// Before setting new columns, see if they changed
bool newColumns = prevColumns != csvColumns;
m_comboModel->setStringList(csvColumns);
// Try to match named columns to the combo boxes
@ -177,9 +187,10 @@ void CsvImportWidget::updatePreview()
break;
}
}
// Named column not found, default to "Not Present"
// Named column not found, default to "Not Present" or previous index
if (!found) {
m_combos.at(i)->setCurrentIndex(0);
auto idx = newColumns ? 0 : prevComboIndexes.at(i);
m_combos.at(i)->setCurrentIndex(idx);
}
}
@ -196,15 +207,19 @@ void CsvImportWidget::load(const QString& filename)
void CsvImportWidget::parse()
{
configParser();
// Hide any previous messages
emit message("");
QApplication::setOverrideCursor(Qt::WaitCursor);
QApplication::processEvents();
bool good = m_parserModel->parse();
updatePreview();
QApplication::restoreOverrideCursor();
if (!good) {
configParser();
if (!m_parserModel->parse()) {
emit message(tr("Failed to parse CSV file: %1").arg(formatStatusText()));
}
updatePreview();
QApplication::restoreOverrideCursor();
}
QSharedPointer<Database> CsvImportWidget::buildDatabase()

120
src/gui/csvImport/CsvImportWidget.ui
View File

@ -79,7 +79,17 @@
</widget>
</item>
<item row="0" column="4">
<widget class="QComboBox" name="notesCombo"/>
<widget class="QComboBox" name="notesCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="0" column="3">
<widget class="QLabel" name="notesLabel">
@ -120,16 +130,56 @@
</widget>
</item>
<item row="4" column="1">
<widget class="QComboBox" name="urlCombo"/>
<widget class="QComboBox" name="urlCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="1" column="4">
<widget class="QComboBox" name="totpCombo"/>
<widget class="QComboBox" name="totpCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="4" column="4">
<widget class="QComboBox" name="createdCombo"/>
<widget class="QComboBox" name="createdCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="1" column="1">
<widget class="QComboBox" name="titleCombo"/>
<widget class="QComboBox" name="titleCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="5" column="0" colspan="5">
<widget class="QCheckBox" name="checkBoxFieldNames">
@ -148,10 +198,30 @@
</widget>
</item>
<item row="2" column="4">
<widget class="QComboBox" name="iconCombo"/>
<widget class="QComboBox" name="iconCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="2" column="1">
<widget class="QComboBox" name="usernameCombo"/>
<widget class="QComboBox" name="usernameCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="3" column="0">
<widget class="QLabel" name="passwordLabel">
@ -192,7 +262,17 @@
</widget>
</item>
<item row="3" column="1">
<widget class="QComboBox" name="passwordCombo"/>
<widget class="QComboBox" name="passwordCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="2" column="0">
<widget class="QLabel" name="usernameLabel">
@ -233,7 +313,17 @@
</widget>
</item>
<item row="3" column="4">
<widget class="QComboBox" name="lastModifiedCombo"/>
<widget class="QComboBox" name="lastModifiedCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="4" column="3">
<widget class="QLabel" name="createdLabel">
@ -255,7 +345,17 @@
</widget>
</item>
<item row="0" column="1">
<widget class="QComboBox" name="groupCombo"/>
<widget class="QComboBox" name="groupCombo">
<property name="maximumSize">
<size>
<width>200</width>
<height>16777215</height>
</size>
</property>
<property name="sizeAdjustPolicy">
<enum>QComboBox::AdjustToContents</enum>
</property>
</widget>
</item>
<item row="3" column="3">
<widget class="QLabel" name="lastModifiedLabel">

6
src/gui/wizard/ImportWizardPageReview.cpp
View File

@ -122,7 +122,11 @@ void ImportWizardPageReview::setupCsvImport(const QString& filename)
m_csvWidget = new CsvImportWidget();
connect(m_csvWidget, &CsvImportWidget::message, m_ui->messageWidget, [this](QString message) {
m_ui->messageWidget->showMessage(message, KMessageWidget::Error, -1);
if (message.isEmpty()) {
m_ui->messageWidget->hideMessage();
} else {
m_ui->messageWidget->showMessage(message, MessageWidget::Error, -1);
}
});
m_csvWidget->load(filename);

159
tests/TestCsvParser.cpp
View File

@ -22,6 +22,18 @@
QTEST_GUILESS_MAIN(TestCsvParser)
void TestCsvParser::writeToFile(const QString& contents)
{
if (!file->open()) {
QFAIL("Cannot open temporary file!");
}
QTextStream out(file.data());
out.setCodec("UTF-8");
out << contents;
out.flush();
file->close();
}
void TestCsvParser::initTestCase()
{
parser.reset(new CsvParser());
@ -30,9 +42,7 @@ void TestCsvParser::initTestCase()
void TestCsvParser::init()
{
file.reset(new QTemporaryFile());
if (not file->open()) {
QFAIL("Cannot open file!");
}
parser->setBackslashSyntax(false);
parser->setComment('#');
parser->setFieldSeparator(',');
@ -47,36 +57,34 @@ void TestCsvParser::cleanup()
/****************** TEST CASES ******************/
void TestCsvParser::testMissingQuote()
{
writeToFile("A,B\n:BM,1");
parser->setTextQualifier(':');
QTextStream out(file.data());
out << "A,B\n:BM,1";
QEXPECT_FAIL("", "Bad format", Continue);
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(!parser->parse(file.data()));
QWARN(parser->getStatus().toLatin1());
}
void TestCsvParser::testMalformed()
{
writeToFile("A,B,C\n:BM::,1,:2:");
parser->setTextQualifier(':');
QTextStream out(file.data());
out << "A,B,C\n:BM::,1,:2:";
QEXPECT_FAIL("", "Bad format", Continue);
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(!parser->parse(file.data()));
QWARN(parser->getStatus().toLatin1());
}
void TestCsvParser::testBackslashSyntax()
{
// attended result: one"\t\"wo
writeToFile("Xone\\\"\\\\t\\\\\\\"w\noX\n"
"X13X,X2\\X,X,\"\"3\"X\r"
"3,X\"4\"X,,\n"
"XX\n"
"\\");
parser->setBackslashSyntax(true);
parser->setTextQualifier(QChar('X'));
QTextStream out(file.data());
// attended result: one"\t\"wo
out << "Xone\\\"\\\\t\\\\\\\"w\noX\n"
<< "X13X,X2\\X,X,\"\"3\"X\r" << "3,X\"4\"X,,\n"
<< "XX\n"
<< "\\";
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.at(0).at(0) == "one\"\\t\\\"w\no");
@ -93,9 +101,9 @@ void TestCsvParser::testBackslashSyntax()
void TestCsvParser::testQuoted()
{
QTextStream out(file.data());
out << "ro,w,\"end, of \"\"\"\"\"\"row\"\"\"\"\"\n"
<< "2\n";
writeToFile("ro,w,\"end, of \"\"\"\"\"\"row\"\"\"\"\"\n"
"2\n");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.at(0).at(0) == "ro");
@ -107,8 +115,6 @@ void TestCsvParser::testQuoted()
void TestCsvParser::testEmptySimple()
{
QTextStream out(file.data());
out << "";
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.isEmpty());
@ -116,8 +122,8 @@ void TestCsvParser::testEmptySimple()
void TestCsvParser::testEmptyQuoted()
{
QTextStream out(file.data());
out << "\"\"";
writeToFile("\"\"");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.isEmpty());
@ -125,8 +131,8 @@ void TestCsvParser::testEmptyQuoted()
void TestCsvParser::testEmptyNewline()
{
QTextStream out(file.data());
out << "\"\n\"";
writeToFile("\"\n\"");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.isEmpty());
@ -141,8 +147,8 @@ void TestCsvParser::testEmptyFile()
void TestCsvParser::testNewline()
{
QTextStream out(file.data());
out << "1,2\n\n\n";
writeToFile("1,2\n\n\n");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 1);
@ -152,8 +158,8 @@ void TestCsvParser::testNewline()
void TestCsvParser::testCR()
{
QTextStream out(file.data());
out << "1,2\r3,4";
writeToFile("1,2\r3,4");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 2);
@ -165,8 +171,8 @@ void TestCsvParser::testCR()
void TestCsvParser::testLF()
{
QTextStream out(file.data());
out << "1,2\n3,4";
writeToFile("1,2\n3,4");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 2);
@ -178,8 +184,8 @@ void TestCsvParser::testLF()
void TestCsvParser::testCRLF()
{
QTextStream out(file.data());
out << "1,2\r\n3,4";
writeToFile("1,2\r\n3,4");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 2);
@ -191,11 +197,12 @@ void TestCsvParser::testCRLF()
void TestCsvParser::testComments()
{
QTextStream out(file.data());
out << " #one\n"
<< " \t # two, three \r\n"
<< " #, sing\t with\r" << " #\t me!\n"
<< "useful,text #1!";
writeToFile(" #one\n"
" \t # two, three \r\n"
" #, sing\t with\r"
" #\t me!\n"
"useful,text #1!");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 1);
@ -205,10 +212,10 @@ void TestCsvParser::testComments()
void TestCsvParser::testColumns()
{
QTextStream out(file.data());
out << "1,2\n"
<< ",,,,,,,,,a\n"
<< "a,b,c,d\n";
writeToFile("1,2\n"
",,,,,,,,,a\n"
"a,b,c,d\n");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(parser->getCsvCols() == 10);
@ -216,10 +223,10 @@ void TestCsvParser::testColumns()
void TestCsvParser::testSimple()
{
QTextStream out(file.data());
out << ",,2\r,2,3\n"
<< "A,,B\"\n"
<< " ,,\n";
writeToFile(",,2\r,2,3\n"
"A,,B\"\n"
" ,,\n");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 4);
@ -239,11 +246,12 @@ void TestCsvParser::testSimple()
void TestCsvParser::testSeparator()
{
writeToFile("\t\t2\r\t2\t3\n"
"A\t\tB\"\n"
" \t\t\n");
parser->setFieldSeparator('\t');
QTextStream out(file.data());
out << "\t\t2\r\t2\t3\n"
<< "A\t\tB\"\n"
<< " \t\t\n";
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 4);
@ -263,10 +271,11 @@ void TestCsvParser::testSeparator()
void TestCsvParser::testMultiline()
{
writeToFile(":1\r\n2a::b:,:3\r4:\n"
"2\n");
parser->setTextQualifier(QChar(':'));
QTextStream out(file.data());
out << ":1\r\n2a::b:,:3\r4:\n"
<< "2\n";
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.at(0).at(0) == "1\n2a:b");
@ -275,41 +284,34 @@ void TestCsvParser::testMultiline()
QVERIFY(t.size() == 2);
}
void TestCsvParser::testEmptyReparsing()
{
parser->parse(nullptr);
QVERIFY(parser->reparse());
t = parser->getCsvTable();
QVERIFY(t.isEmpty());
}
void TestCsvParser::testReparsing()
{
QTextStream out(file.data());
out << ":te\r\nxt1:,:te\rxt2:,:end of \"this\n string\":\n"
<< "2\n";
writeToFile(":te\r\nxt1:,:te\rxt2:,:end of \"this\n string\":\n"
"2\n");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QEXPECT_FAIL("", "Wrong qualifier", Continue);
QVERIFY(t.at(0).at(0) == "te\nxt1");
QCOMPARE(t.at(0).at(0), QString(":te"));
parser->setTextQualifier(QChar(':'));
QVERIFY(parser->reparse());
t = parser->getCsvTable();
QVERIFY(t.at(0).at(0) == "te\nxt1");
QVERIFY(t.at(0).at(1) == "te\nxt2");
QVERIFY(t.at(0).at(2) == "end of \"this\n string\"");
QVERIFY(t.at(1).at(0) == "2");
QVERIFY(t.size() == 2);
QCOMPARE(t.at(0).at(0), QString("te\nxt1"));
QCOMPARE(t.at(0).at(1), QString("te\nxt2"));
QCOMPARE(t.at(0).at(2), QString("end of \"this\n string\""));
QCOMPARE(t.at(1).at(0), QString("2"));
QCOMPARE(t.size(), 2);
}
void TestCsvParser::testQualifier()
{
writeToFile("X1X,X2XX,X,\"\"3\"\"\"X\r"
"3,X\"4\"X,,\n");
parser->setTextQualifier(QChar('X'));
QTextStream out(file.data());
out << "X1X,X2XX,X,\"\"3\"\"\"X\r" << "3,X\"4\"X,,\n";
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();
QVERIFY(t.size() == 2);
@ -328,10 +330,9 @@ void TestCsvParser::testUnicode()
// CORRECT QString g("\u20AC");
// CORRECT QChar g(0x20AC);
// ERROR QChar g("\u20AC");
writeToFile("€1A2śA\"3śAż\"Ażac");
parser->setFieldSeparator(QChar('A'));
QTextStream out(file.data());
out.setCodec("UTF-8");
out << QString("€1A2śA\"3śAż\"Ażac");
QVERIFY(parser->parse(file.data()));
t = parser->getCsvTable();

3
tests/TestCsvParser.h
View File

@ -37,7 +37,6 @@ private slots:
void testUnicode();
void testLF();
void testEmptyReparsing();
void testSimple();
void testEmptyQuoted();
void testEmptyNewline();
@ -58,6 +57,8 @@ private slots:
void testColumns();
private:
void writeToFile(const QString& contents);
QScopedPointer<QTemporaryFile> file;
QScopedPointer<CsvParser> parser;
CsvTable t;