Skip to content

Commit

Permalink
Fix wrong behavior when reading text
Browse files Browse the repository at this point in the history
Also add another 'file read error' status.

Closes qbittorrent#19254.
PR qbittorrent#19262.
  • Loading branch information
Chocobo1 authored Jul 2, 2023
1 parent 08a7714 commit 80791e3
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 11 deletions.
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ core.eol=lf
*.png binary
*.qm binary
*.zip binary

test/testdata/crlf.txt text eol=crlf
8 changes: 4 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,19 @@ repos:
args: ["--fix=lf"]
exclude: |
(?x)^(
compile_commands.json |
src/webui/www/private/css/lib/.* |
src/webui/www/private/scripts/lib/.*
src/webui/www/private/scripts/lib/.* |
test/testdata/crlf.txt
)$
- id: end-of-file-fixer
name: Check trailing newlines
exclude: |
(?x)^(
compile_commands.json |
configure |
src/webui/www/private/css/lib/.* |
src/webui/www/private/scripts/lib/.*
src/webui/www/private/scripts/lib/.* |
test/testdata/crlf.txt
)$
exclude_types:
- svg
Expand Down
35 changes: 28 additions & 7 deletions src/base/utils/io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,37 @@ nonstd::expected<QByteArray, Utils::IO::ReadError> Utils::IO::readFile(const Pat
return nonstd::make_unexpected(ReadError {ReadError::ExceedSize, message});
}

// Do not use `QIODevice::readAll()` it won't stop when reading `/dev/zero`
const QByteArray data = file.read(fileSize);
if (const qint64 dataSize = data.size(); dataSize != fileSize)
#if (QT_VERSION >= QT_VERSION_CHECK(6, 5, 0))
QByteArray ret {fileSize, Qt::Uninitialized};
#else
QByteArray ret {static_cast<int>(fileSize), Qt::Uninitialized};
#endif
const qint64 actualSize = file.read(ret.data(), fileSize);

if (actualSize < 0)
{
const QString message = QCoreApplication::translate("Utils::IO", "File read error. File: \"%1\". Error: \"%2\"")
.arg(file.fileName(), file.errorString());
return nonstd::make_unexpected(ReadError {ReadError::Failed, message});
}

if (actualSize < fileSize)
{
const QString message = QCoreApplication::translate("Utils::IO", "Read size mismatch. File: \"%1\". Expected: %2. Actual: %3")
.arg(file.fileName(), QString::number(fileSize), QString::number(dataSize));
return nonstd::make_unexpected(ReadError {ReadError::SizeMismatch, message});
// `QIODevice::Text` will convert CRLF to LF on-the-fly and affects return value
// of `qint64 QIODevice::read(char *data, qint64 maxSize)`
if (additionalMode.testFlag(QIODevice::Text))
{
ret.truncate(actualSize);
}
else
{
const QString message = QCoreApplication::translate("Utils::IO", "Read size mismatch. File: \"%1\". Expected: %2. Actual: %3")
.arg(file.fileName(), QString::number(fileSize), QString::number(actualSize));
return nonstd::make_unexpected(ReadError {ReadError::SizeMismatch, message});
}
}

return data;
return ret;
}

nonstd::expected<void, QString> Utils::IO::saveToFile(const Path &path, const QByteArray &data)
Expand Down
1 change: 1 addition & 0 deletions src/base/utils/io.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ namespace Utils::IO
{
NotExist,
ExceedSize,
Failed, // `read()` operation failed
SizeMismatch
};

Expand Down
1 change: 1 addition & 0 deletions src/webui/webapplication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ void WebApplication::sendFile(const Path &path)
LogMsg(message, Log::WARNING);
throw InternalServerErrorHTTPError(readResult.error().message);

case Utils::IO::ReadError::Failed:
case Utils::IO::ReadError::SizeMismatch:
LogMsg(message, Log::WARNING);
throw InternalServerErrorHTTPError(readResult.error().message);
Expand Down
2 changes: 2 additions & 0 deletions test/testdata/crlf.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@


7 changes: 7 additions & 0 deletions test/testutilsio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ private slots:
QCOMPARE(readResult.value(), size10Data);
}

{
const Path crlfFile = testFolder / Path(u"crlf.txt"_s);
const auto readResult = Utils::IO::readFile(crlfFile, -1, QIODevice::Text);
QCOMPARE(readResult.has_value(), true);
QCOMPARE(readResult.value(), "\n\n");
}

{
const Path nonExistFile = testFolder / Path(u".non_existent_file_1234"_s);
const auto readResult = Utils::IO::readFile(nonExistFile, 1);
Expand Down

0 comments on commit 80791e3

Please sign in to comment.