Skip to content

Commit

Permalink
Read UTF-16 files through Qt decoding to a temporary file
Browse files Browse the repository at this point in the history
Fixes OpenChemistry/avogadrolibs#1689

Signed-off-by: Geoff Hutchison <[email protected]>
  • Loading branch information
ghutchis committed Dec 31, 2024
1 parent 5d8a673 commit eedf399
Showing 1 changed file with 58 additions and 5 deletions.
63 changes: 58 additions & 5 deletions avogadro/backgroundfileformat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@

#include <avogadro/io/fileformat.h>

#include <QtCore/QFile>
#include <QtCore/QTemporaryDir>
#include <QtCore/QTextStream>

namespace Avogadro {

BackgroundFileFormat::BackgroundFileFormat(Io::FileFormat* format,
QObject* aparent)
: QObject(aparent), m_format(format), m_molecule(nullptr), m_success(false)
: QObject(aparent)
, m_format(format)
, m_molecule(nullptr)
, m_success(false)
{
}

Expand All @@ -35,8 +42,54 @@ void BackgroundFileFormat::read()
m_error = tr("No file name set in BackgroundFileFormat!");

if (m_error.isEmpty()) {
m_success = m_format->readFile(m_fileName.toLocal8Bit().data(),
*m_molecule);
// sometimes we hit UTF-16 files, so we need to convert them to UTF-8
// first check whether the file is UTF-16
QFile file(m_fileName);
QTextStream in(&file);
QString text;
bool isUTF16 = false;
if (file.open(QIODevice::ReadOnly)) {

Check notice

Code scanning / Flawfinder (reported by Codacy)

Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362). Note

Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362).
QByteArray data = file.read(2);

Check notice

Code scanning / Flawfinder (reported by Codacy)

Check buffer boundaries if used in a loop including recursive loops (CWE-120, CWE-20). Note

Check buffer boundaries if used in a loop including recursive loops (CWE-120, CWE-20).
// look for a byte-order mark
if ((data.size() == 2 && data[0] == '\xff' && data[1] == '\xfe') ||
(data.size() == 2 && data[0] == '\xfe' && data[1] == '\xff')) {
// UTF-16, read the file and let QString handle decoding
isUTF16 = true;
file.close();
file.open(QIODevice::ReadOnly | QIODevice::Text);

Check notice

Code scanning / Flawfinder (reported by Codacy)

Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362). Note

Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362).
in.setCodec("UTF-16");
text = in.readAll();
file.close();
}
}

if (!isUTF16)
m_success =
m_format->readFile(m_fileName.toLocal8Bit().data(), *m_molecule);
else {
// write it to a temporary file and we'll read it back in
// some formats (like the generic output) need a file
// not just a string bugger

// first, we need the *name* of the file, not the full path
// because we're going to save a copy in a temp directory
QTemporaryDir tempDir;
QString tempFileName = tempDir.filePath(QFileInfo(m_fileName).fileName());
QFile tempFile(tempFileName);
if (tempFile.open(QIODevice::WriteOnly | QIODevice::Text)) {

Check notice

Code scanning / Flawfinder (reported by Codacy)

Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362). Note

Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362).
QTextStream out(&tempFile);
// set it to UTF-8
out.setCodec("UTF-8");
out << text;
out.flush();
tempFile.close();
m_success =
m_format->readFile(tempFileName.toLocal8Bit().data(), *m_molecule);
tempFile.remove();
} else // try just reading the string
m_success =
m_format->readString(text.toLocal8Bit().data(), *m_molecule);
}

if (!m_success)
m_error = QString::fromStdString(m_format->error());
Expand All @@ -60,8 +113,8 @@ void BackgroundFileFormat::write()
m_error = tr("No file name set in BackgroundFileFormat!");

if (m_error.isEmpty()) {
m_success = m_format->writeFile(m_fileName.toLocal8Bit().data(),
*m_molecule);
m_success =
m_format->writeFile(m_fileName.toLocal8Bit().data(), *m_molecule);

if (!m_success)
m_error = QString::fromStdString(m_format->error());
Expand Down

0 comments on commit eedf399

Please sign in to comment.