From 7263dcddfe52552747c478da941f1f078d8d91e0 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sun, 28 Oct 2018 19:55:00 +0100 Subject: [PATCH] Fix stdin/stdout encoding on Windows. (#2425) QTextStream uses the system default locale, but this breaks in various situations: (1) It does not work on the native Windows shell (cmd.exe, Powershell), since the default Windows locale is Windows-1252, but the shell uses Windows-850. (2) It also breaks on *nix systems where the locale is Latin1 or C, which is the case for most CI systems or build servers. We allow overriding the detected codec by setting the ENCODING_OVERRIDE environment variable, but otherwise prefer Windows-850 on Windows and UTF-8 on any other system, even if LANG is set to something else. This resolves #2413 --- src/CMakeLists.txt | 3 +- src/cli/Add.cpp | 8 ++--- src/cli/Clip.cpp | 8 ++--- src/cli/Diceware.cpp | 6 ++-- src/cli/Edit.cpp | 8 ++--- src/cli/Estimate.cpp | 8 ++--- src/cli/Extract.cpp | 6 ++-- src/cli/Generate.cpp | 10 +++--- src/cli/List.cpp | 8 ++--- src/cli/Locate.cpp | 10 +++--- src/cli/Merge.cpp | 6 ++-- src/cli/Remove.cpp | 8 ++--- src/cli/Show.cpp | 10 +++--- src/cli/TextStream.cpp | 73 +++++++++++++++++++++++++++++++++++++++ src/cli/TextStream.h | 50 +++++++++++++++++++++++++++ src/cli/Utils.cpp | 6 ++-- src/cli/Utils.h | 2 +- src/cli/keepassxc-cli.cpp | 4 +-- tests/TestCli.cpp | 7 ++-- 19 files changed, 180 insertions(+), 61 deletions(-) create mode 100644 src/cli/TextStream.cpp create mode 100644 src/cli/TextStream.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 37fe0b553..c6f614568 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -70,10 +70,9 @@ set(keepassx_SOURCES core/Clock.cpp core/Tools.cpp core/Translator.cpp - core/Base32.h core/Base32.cpp cli/Utils.cpp - cli/Utils.h + cli/TextStream.cpp crypto/Crypto.cpp crypto/CryptoHash.cpp crypto/Random.cpp diff --git a/src/cli/Add.cpp b/src/cli/Add.cpp index 81a5cad13..09a161071 100644 --- a/src/cli/Add.cpp +++ b/src/cli/Add.cpp @@ -21,8 +21,8 @@ #include "Add.h" #include -#include +#include "cli/TextStream.h" #include "cli/Utils.h" #include "core/Database.h" #include "core/Entry.h" @@ -41,9 +41,9 @@ Add::~Add() int Add::execute(const QStringList& arguments) { - QTextStream inputTextStream(Utils::STDIN, QIODevice::ReadOnly); - QTextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly); + TextStream inputTextStream(Utils::STDIN, QIODevice::ReadOnly); + TextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly); + TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); diff --git a/src/cli/Clip.cpp b/src/cli/Clip.cpp index 0b78a24b4..f04c8b5e9 100644 --- a/src/cli/Clip.cpp +++ b/src/cli/Clip.cpp @@ -23,8 +23,8 @@ #include "Clip.h" #include -#include +#include "cli/TextStream.h" #include "cli/Utils.h" #include "core/Database.h" #include "core/Entry.h" @@ -42,7 +42,7 @@ Clip::~Clip() int Clip::execute(const QStringList& arguments) { - QTextStream out(Utils::STDOUT); + TextStream out(Utils::STDOUT); QCommandLineParser parser; parser.setApplicationDescription(description); @@ -73,7 +73,7 @@ int Clip::execute(const QStringList& arguments) int Clip::clipEntry(Database* database, QString entryPath, QString timeout) { - QTextStream err(Utils::STDERR); + TextStream err(Utils::STDERR); int timeoutSeconds = 0; if (!timeout.isEmpty() && !timeout.toInt()) { @@ -83,7 +83,7 @@ int Clip::clipEntry(Database* database, QString entryPath, QString timeout) timeoutSeconds = timeout.toInt(); } - QTextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly); + TextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly); Entry* entry = database->rootGroup()->findEntry(entryPath); if (!entry) { err << QObject::tr("Entry %1 not found.").arg(entryPath) << endl; diff --git a/src/cli/Diceware.cpp b/src/cli/Diceware.cpp index 72cbd1960..d2821e7cc 100644 --- a/src/cli/Diceware.cpp +++ b/src/cli/Diceware.cpp @@ -21,8 +21,8 @@ #include "Diceware.h" #include -#include +#include "cli/TextStream.h" #include "core/PassphraseGenerator.h" #include "Utils.h" @@ -38,8 +38,8 @@ Diceware::~Diceware() int Diceware::execute(const QStringList& arguments) { - QTextStream in(Utils::STDIN, QIODevice::ReadOnly); - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream in(Utils::STDIN, QIODevice::ReadOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); diff --git a/src/cli/Edit.cpp b/src/cli/Edit.cpp index c2f067794..91a76b195 100644 --- a/src/cli/Edit.cpp +++ b/src/cli/Edit.cpp @@ -21,8 +21,8 @@ #include "Edit.h" #include -#include +#include "cli/TextStream.h" #include "cli/Utils.h" #include "core/Database.h" #include "core/Entry.h" @@ -41,9 +41,9 @@ Edit::~Edit() int Edit::execute(const QStringList& arguments) { - QTextStream in(Utils::STDIN, QIODevice::ReadOnly); - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream err(Utils::STDERR, QIODevice::WriteOnly); + TextStream in(Utils::STDIN, QIODevice::ReadOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream err(Utils::STDERR, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); diff --git a/src/cli/Estimate.cpp b/src/cli/Estimate.cpp index c249d7b1f..db32f8dc7 100644 --- a/src/cli/Estimate.cpp +++ b/src/cli/Estimate.cpp @@ -19,8 +19,8 @@ #include "cli/Utils.h" #include -#include +#include "cli/TextStream.h" #include #include #include @@ -45,7 +45,7 @@ Estimate::~Estimate() static void estimate(const char* pwd, bool advanced) { - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); double e = 0.0; int len = static_cast(strlen(pwd)); @@ -150,8 +150,8 @@ static void estimate(const char* pwd, bool advanced) int Estimate::execute(const QStringList& arguments) { - QTextStream in(Utils::STDIN, QIODevice::ReadOnly); - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream in(Utils::STDIN, QIODevice::ReadOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); diff --git a/src/cli/Extract.cpp b/src/cli/Extract.cpp index cc39c469a..32b1bc028 100644 --- a/src/cli/Extract.cpp +++ b/src/cli/Extract.cpp @@ -22,8 +22,8 @@ #include #include -#include +#include "cli/TextStream.h" #include "cli/Utils.h" #include "core/Database.h" #include "format/KeePass2Reader.h" @@ -43,8 +43,8 @@ Extract::~Extract() int Extract::execute(const QStringList& arguments) { - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream err(Utils::STDERR, QIODevice::WriteOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream err(Utils::STDERR, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); diff --git a/src/cli/Generate.cpp b/src/cli/Generate.cpp index 7780aa829..da2deb8d8 100644 --- a/src/cli/Generate.cpp +++ b/src/cli/Generate.cpp @@ -22,8 +22,8 @@ #include "cli/Utils.h" #include -#include +#include "cli/TextStream.h" #include "core/PasswordGenerator.h" Generate::Generate() @@ -38,9 +38,8 @@ Generate::~Generate() int Generate::execute(const QStringList& arguments) { - QTextStream in(Utils::STDIN, QIODevice::ReadOnly); - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - out.setCodec("UTF-8"); // force UTF-8 to prevent ??? characters in extended-ASCII passwords + TextStream in(Utils::STDIN, QIODevice::ReadOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); @@ -87,8 +86,7 @@ int Generate::execute(const QStringList& arguments) if (parser.value(len).isEmpty()) { passwordGenerator.setLength(PasswordGenerator::DefaultLength); } else { - int length = parser.value(len).toInt(); - passwordGenerator.setLength(static_cast(length)); + passwordGenerator.setLength(parser.value(len).toInt()); } PasswordGenerator::CharClasses classes = 0x0; diff --git a/src/cli/List.cpp b/src/cli/List.cpp index 4d1ebcfc5..6d85404ff 100644 --- a/src/cli/List.cpp +++ b/src/cli/List.cpp @@ -22,8 +22,8 @@ #include "cli/Utils.h" #include -#include +#include "cli/TextStream.h" #include "core/Database.h" #include "core/Entry.h" #include "core/Group.h" @@ -40,7 +40,7 @@ List::~List() int List::execute(const QStringList& arguments) { - QTextStream out(Utils::STDOUT); + TextStream out(Utils::STDOUT); QCommandLineParser parser; parser.setApplicationDescription(description); @@ -77,8 +77,8 @@ int List::execute(const QStringList& arguments) int List::listGroup(Database* database, bool recursive, const QString& groupPath) { - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream err(Utils::STDERR, QIODevice::WriteOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream err(Utils::STDERR, QIODevice::WriteOnly); if (groupPath.isEmpty()) { out << database->rootGroup()->print(recursive) << flush; diff --git a/src/cli/Locate.cpp b/src/cli/Locate.cpp index 3bca8ae1d..8ab8f4c61 100644 --- a/src/cli/Locate.cpp +++ b/src/cli/Locate.cpp @@ -1,5 +1,3 @@ -#include - /* * Copyright (C) 2017 KeePassXC Team * @@ -24,8 +22,8 @@ #include #include -#include +#include "cli/TextStream.h" #include "cli/Utils.h" #include "core/Global.h" #include "core/Database.h" @@ -44,7 +42,7 @@ Locate::~Locate() int Locate::execute(const QStringList& arguments) { - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); @@ -73,8 +71,8 @@ int Locate::execute(const QStringList& arguments) int Locate::locateEntry(Database* database, const QString& searchTerm) { - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream err(Utils::STDERR, QIODevice::WriteOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream err(Utils::STDERR, QIODevice::WriteOnly); QStringList results = database->rootGroup()->locate(searchTerm); if (results.isEmpty()) { diff --git a/src/cli/Merge.cpp b/src/cli/Merge.cpp index a5b4a2cb7..e81b7aa99 100644 --- a/src/cli/Merge.cpp +++ b/src/cli/Merge.cpp @@ -18,8 +18,8 @@ #include "Merge.h" #include -#include +#include "cli/TextStream.h" #include "core/Database.h" #include "core/Merger.h" #include "cli/Utils.h" @@ -38,8 +38,8 @@ Merge::~Merge() int Merge::execute(const QStringList& arguments) { - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream err(Utils::STDERR, QIODevice::WriteOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream err(Utils::STDERR, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(description); diff --git a/src/cli/Remove.cpp b/src/cli/Remove.cpp index 6523cff97..4800b5c94 100644 --- a/src/cli/Remove.cpp +++ b/src/cli/Remove.cpp @@ -23,8 +23,8 @@ #include #include #include -#include +#include "cli/TextStream.h" #include "cli/Utils.h" #include "core/Database.h" #include "core/Entry.h" @@ -44,7 +44,7 @@ Remove::~Remove() int Remove::execute(const QStringList& arguments) { - QTextStream out(Utils::STDERR, QIODevice::WriteOnly); + TextStream out(Utils::STDERR, QIODevice::WriteOnly); QCommandLineParser parser; parser.setApplicationDescription(QCoreApplication::tr("main", "Remove an entry from the database.")); @@ -73,8 +73,8 @@ int Remove::execute(const QStringList& arguments) int Remove::removeEntry(Database* database, const QString& databasePath, const QString& entryPath) { - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream err(Utils::STDERR, QIODevice::WriteOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream err(Utils::STDERR, QIODevice::WriteOnly); QPointer entry = database->rootGroup()->findEntryByPath(entryPath); if (!entry) { diff --git a/src/cli/Show.cpp b/src/cli/Show.cpp index 5e2ec14b4..032f1fcad 100644 --- a/src/cli/Show.cpp +++ b/src/cli/Show.cpp @@ -21,8 +21,8 @@ #include #include -#include +#include "cli/TextStream.h" #include "core/Database.h" #include "core/Entry.h" #include "core/Group.h" @@ -41,7 +41,7 @@ Show::~Show() int Show::execute(const QStringList& arguments) { - QTextStream out(Utils::STDOUT); + TextStream out(Utils::STDOUT); QCommandLineParser parser; parser.setApplicationDescription(description); @@ -78,9 +78,9 @@ int Show::execute(const QStringList& arguments) int Show::showEntry(Database* database, QStringList attributes, const QString& entryPath) { - QTextStream in(Utils::STDIN, QIODevice::ReadOnly); - QTextStream out(Utils::STDOUT, QIODevice::WriteOnly); - QTextStream err(Utils::STDERR, QIODevice::WriteOnly); + TextStream in(Utils::STDIN, QIODevice::ReadOnly); + TextStream out(Utils::STDOUT, QIODevice::WriteOnly); + TextStream err(Utils::STDERR, QIODevice::WriteOnly); Entry* entry = database->rootGroup()->findEntry(entryPath); if (!entry) { diff --git a/src/cli/TextStream.cpp b/src/cli/TextStream.cpp new file mode 100644 index 000000000..d75cb74a9 --- /dev/null +++ b/src/cli/TextStream.cpp @@ -0,0 +1,73 @@ +/* + * Copyright (C) 2018 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "TextStream.h" + +#include +#include + +TextStream::TextStream() +{ + detectCodec(); +} + +TextStream::TextStream(QIODevice* device) + : QTextStream(device) +{ + detectCodec(); +} + +TextStream::TextStream(FILE* fileHandle, QIODevice::OpenMode openMode) + : QTextStream(fileHandle, openMode) +{ + detectCodec(); +} + +TextStream::TextStream(QString* string, QIODevice::OpenMode openMode) + : QTextStream(string, openMode) +{ + detectCodec(); +} + +TextStream::TextStream(QByteArray* array, QIODevice::OpenMode openMode) + : QTextStream(array, openMode) +{ + detectCodec(); +} + +TextStream::TextStream(const QByteArray& array, QIODevice::OpenMode openMode) + : QTextStream(array, openMode) +{ + detectCodec(); +} + +void TextStream::detectCodec() +{ + QString codecName = "UTF-8"; + auto env = QProcessEnvironment::systemEnvironment(); +#ifdef Q_OS_WIN + if (!env.contains("SHELL")) { + // native shell (no Msys or cygwin) + codecName = "Windows-850"; + } +#endif + codecName = env.value("ENCODING_OVERRIDE", codecName); + auto* codec = QTextCodec::codecForName(codecName.toLatin1()); + if (codec) { + setCodec(codec); + } +} diff --git a/src/cli/TextStream.h b/src/cli/TextStream.h new file mode 100644 index 000000000..971f8651d --- /dev/null +++ b/src/cli/TextStream.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSXC_TEXTSTREAM_H +#define KEEPASSXC_TEXTSTREAM_H + +#include + +/** + * QTextStream with codec fixes for the Windows command line. + * + * QTextStream uses the system default locale, but this breaks in various + * situations: (1) It does not work on the native Windows shell (cmd.exe, Powershell), + * since the default Windows locale is Windows-1252, but the shell uses Windows-850. + * (2) It also breaks on *nix systems where the locale is Latin1 or C, which + * is the case for most CI systems or build servers. + * + * We allow overriding the detected codec by setting the ENCODING_OVERRIDE + * environment variable, but otherwise prefer Windows-850 on Windows and UTF-8 + * on any other system, even if LANG is set to something else. + */ +class TextStream : public QTextStream +{ +public: + TextStream(); + explicit TextStream(QIODevice* device); + explicit TextStream(FILE* fileHandle, QIODevice::OpenMode openMode = QIODevice::ReadWrite); + explicit TextStream(QString* string, QIODevice::OpenMode openMode = QIODevice::ReadWrite); + explicit TextStream(QByteArray* array, QIODevice::OpenMode openMode = QIODevice::ReadWrite); + explicit TextStream(const QByteArray& array, QIODevice::OpenMode openMode = QIODevice::ReadOnly); + +private: + void detectCodec(); +}; + +#endif //KEEPASSXC_TEXTSTREAM_H diff --git a/src/cli/Utils.cpp b/src/cli/Utils.cpp index a0f75bc8e..a7bf83c13 100644 --- a/src/cli/Utils.cpp +++ b/src/cli/Utils.cpp @@ -97,7 +97,7 @@ void setNextPassword(const QString& password) */ QString getPassword() { - QTextStream out(STDOUT, QIODevice::WriteOnly); + TextStream out(STDOUT, QIODevice::WriteOnly); // return preset password if one is set if (!Test::nextPasswords.isEmpty()) { @@ -107,7 +107,7 @@ QString getPassword() return password; } - QTextStream in(STDIN, QIODevice::ReadOnly); + TextStream in(STDIN, QIODevice::ReadOnly); setStdinEcho(false); QString line = in.readLine(); @@ -123,7 +123,7 @@ QString getPassword() */ int clipText(const QString& text) { - QTextStream err(Utils::STDERR); + TextStream err(Utils::STDERR); QString programName = ""; QStringList arguments; diff --git a/src/cli/Utils.h b/src/cli/Utils.h index 1d5e0f356..a5c995f10 100644 --- a/src/cli/Utils.h +++ b/src/cli/Utils.h @@ -19,7 +19,7 @@ #define KEEPASSXC_UTILS_H #include -#include +#include "cli/TextStream.h" namespace Utils { diff --git a/src/cli/keepassxc-cli.cpp b/src/cli/keepassxc-cli.cpp index 041908663..a2399e741 100644 --- a/src/cli/keepassxc-cli.cpp +++ b/src/cli/keepassxc-cli.cpp @@ -20,8 +20,8 @@ #include #include #include -#include +#include "cli/TextStream.h" #include #include "config-keepassx.h" @@ -46,7 +46,7 @@ int main(int argc, char** argv) Bootstrap::bootstrapApplication(); #endif - QTextStream out(stdout); + TextStream out(stdout); QStringList arguments; for (int i = 0; i < argc; ++i) { arguments << QString(argv[i]); diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index e10a651d7..c95b1f32b 100644 --- a/tests/TestCli.cpp +++ b/tests/TestCli.cpp @@ -455,12 +455,13 @@ void TestCli::testGenerate() qint64 pos = 0; // run multiple times to make accidental passes unlikely + TextStream stream(m_stdoutFile.data()); for (int i = 0; i < 10; ++i) { generateCmd.execute(parameters); - m_stdoutFile->seek(pos); + stream.seek(pos); QRegularExpression regex(pattern); - QString password = QString::fromUtf8(m_stdoutFile->readLine()); - pos = m_stdoutFile->pos(); + QString password = stream.readLine(); + pos = stream.pos(); QVERIFY2(regex.match(password).hasMatch(), qPrintable("Password " + password + " does not match pattern " + pattern)); } }