QProcess argument encoding issue...?

[mbruel]

Hi,
I'm fighting for few hours on this one: I'd like to compress some files using rar as an external command and using a password.
The command is simple:

rar a -inul -m0 compressed.rar -hp"myPassword" file_to_compress

Everything seems to work.
BUT when I try to unrar my compressed.rar, then the password is incorrect...

I output the command that I'm doing in the QProcess.
If I run it in a terminal, then it is working AND I can decompress using the password.
So it seems to me there is an issue with QProcess passing the password argument to the rar command...
I've tried several things but I couldn't manage to figure out a way to bypass it.
HELP please!

Here is a compiling code to illustrate the issue: (main.cpp)

#include <QProcess>
#include <QTextCodec>
#include <QTextStream>

static const char *rar = "/usr/bin/rar";

int compressFiles(QTextStream &cout,
                  const QString &cmdRar,
                  const QString &archiveName,
                  const QStringList &files,
                  const QString &pass,
                  const QString &compressLevel = "-m0")
{
    QStringList args = {"a", "-inul", compressLevel, archiveName};
    if (!pass.isEmpty())
        args << QString::fromLocal8Bit(QString("-hp\"%1\"").arg(pass).toUtf8());
    args << files;

    cout << QString("Compressing files: %1 %2\n").arg(cmdRar).arg(args.join(" "));
    QProcess proc;
    proc.start(cmdRar, args);
    proc.waitForFinished(-1);
    int exitCode = proc.exitCode();
    cout << QString("rar exit code: %1\n").arg(exitCode);

    return exitCode;
}

#include <QFileInfo>
int main(int argc, char *argv[])
{
    //    QTextCodec *codec = QTextCodec::codecForName("UTF-8");
    //    QTextCodec::setCodecForLocale(codec);

    QTextStream cout(stdout);

    if (argc != 3){
        cout << "Syntax: " << argv[0] << " <file to compress> <password>\n";
        return 1;
    }

    QFileInfo fileInf(argv[1]);
    if (!fileInf.isFile() || !fileInf.isReadable())
    {
        cout << "Error: the file '" << argv[1] << "' is not readable...\n";
        return 1;

    }

    return compressFiles(cout, rar, QString("%1.rar").arg(argv[1]), {argv[1]}, argv[2]);
}

Here qunrar. pro:

QT -= gui
CONFIG += c++11 console
CONFIG -= app_bundle
SOURCES += main.cpp

# Default rules for deployment.
qnx: target.path = /tmp/$${TARGET}/bin
else: unix:!android: target.path = /opt/$${TARGET}/bin
!isEmpty(target.path): INSTALLS += target

So basically here is the problem:

/tmp$ ./qunrar nzbget-21.0-bin-linux.run qwerty
Compressing files: /usr/bin/rar a -inul -m0 nzbget-21.0-bin-linux.run.rar -hp"qwerty" nzbget-21.0-bin-linux.run
rar exit code: 0
/tmp$ unrar x nzbget-21.0-bin-linux.run.rar -pqwerty

UNRAR 5.61 beta 1 freeware      Copyright (c) 1993-2018 Alexander Roshal

The specified password is incorrect.
Total errors: 1


/tmp$ rm nzbget-21.0-bin-linux.run.rar 
/tmp$ /usr/bin/rar a -inul -m0 nzbget-21.0-bin-linux.run.rar -hp"qwerty" nzbget-21.0-bin-linux.run
/tmp$ unrar x nzbget-21.0-bin-linux.run.rar -pqwerty

UNRAR 5.61 beta 1 freeware      Copyright (c) 1993-2018 Alexander Roshal

Extracting from nzbget-21.0-bin-linux.run.rar

Would you like to replace the existing file nzbget-21.0-bin-linux.run
31534532 bytes, modified on 2020-01-06 08:16
with a new one
31534532 bytes, modified on 2020-01-06 08:16

[Y]es, [N]o, [A]ll, n[E]ver, [R]ename, [Q]uit A

Extracting  nzbget-21.0-bin-linux.run                                 OK 
All OK

PS: I'm using Linux (Debian 10.2 64bit) with Qt v5.12.6

[aha_1980]

@mbruel said in QProcess argument encoding issue...?:

QString::fromLocal8Bit(QString("-hp"%1"").arg(pass).toUtf8()

What do you expect this line to generate?

Best would be to stick to ASCII for passwords to avoid all problems with encodings.

On the other hand, args <<QString("-hp\"%1\"").arg(pass) could just work.

Btw: Which platform and Qt version are we talking about?

Because Linux&Mac is all UTF-8 nowadays, Windows is still an encoding disaster.

Regards

[JonB]

@mbruel
I'm not sure, but but I think your
args << QString::fromLocal8Bit(QString("-hp\"%1\"").arg(pass).toUtf8());
is erroneous.

I have a feeling you will find you are setting the password to "qwerty" rather than qwerty.

You could verify this by trying on your Qt-compressed file:
unrar x nzbget-21.0-bin-linux.run.rar '-p"qwerty"'

Try
args << QString::fromLocal8Bit(QString("-hp%1").arg(pass).toUtf8());
that should work. [Though see @aha_1980's observation about this code needing changing.]

If you are passing arguments to QProcess() it is not your job to quote them. QProcess will do that as necessary, according to OS rules for individual arguments as required. Your need to quote when typing into a shell is to do with shell rules. Your QProcess I don't think will even go via shell (more like a straight vfork/exec()), so there must be no quoting.

Am I right?

EDIT
@aha_1980
My answer has crossed with yours. I agree about the UTF conversion code being incorrect, but I still think the password must not be quoted via a QProcess argument.

I look forward to hearing back from @mbruel :)

[mbruel]

@aha_1980 said in QProcess argument encoding issue...?:

What do you expect this line to generate?

Not much, just tired and saw this kind of trick on the web (cf here) but yeah I'm quite aware it should be the same than

args <<QString("-hp\"%1\"").arg(pass)

and that what I had at the beginning. It is not working...

@aha_1980 said in QProcess argument encoding issue...?:

Btw: Which platform and Qt version are we talking about?

As I said totally at the bottom: I'm using Linux (Debian 10.2 64bit) with Qt v5.12.6

@JonB said in QProcess argument encoding issue...?:

Am I right?

yes, you're right! \o/ Thanks a lot man!
well that is the syntax on the shell to add the quotes as the password may contain a space.
But as you said, not our job to add them, Qt manage it.
So changing the code to be the following works like a charm:

        args << QString("-hp%1").arg(pass);

even if I try to introduce space in my password like that:

$ ./qunrar nzbget-21.0-bin-linux.run "qwerty 123"
Compressing files: /usr/bin/rar a -inul -m0 nzbget-21.0-bin-linux.run.rar -hpqwerty 123 nzbget-21.0-bin-linux.run
rar exit code: 0
$ unrar x nzbget-21.0-bin-linux.run.rar -p"qwerty 123"
UNRAR 5.61 beta 1 freeware      Copyright (c) 1993-2018 Alexander Roshal
Extracting  nzbget-21.0-bin-linux.run                                 OK 
All OK

Thanks again!

[JonB]

@mbruel
Yes. it is the job of QProcess separate-argument-processing to deal with any quoting necessary to make each argument correct.

Actually I would have thought you would find: under Windows if Qt calls CreateProcess it (Qt) will have to quote your arguments. Under Linux I would expect it (Qt) to go via an exec...()-type function, where no quoting is done, arguments are passed as separate items to the OS.

P.S.
Just to be 100% clear. The need to not quote is because you are using this overload:
https://doc.qt.io/qt-5/qprocess.html#start, void QProcess::start(const QString &program, const QStringList &arguments, QIODevice::OpenMode mode = ReadWrite).
If you built your own string for the whole command and used this overload:
https://doc.qt.io/qt-5/qprocess.html#start-1, QProcess::start(const QString &command, QIODevice::OpenMode mode = ReadWrite)
then you would need to do your argument quoting, and QProcess would, I think, pass the whole string to the shell instead of the underlying OS call (or do argument splitting itself).

And on a completely unrelated matter. With recent-ish (or has it always been there?) Qt you could replace your lines

    QProcess proc;
    proc.start(cmdRar, args);
    proc.waitForFinished(-1);
    int exitCode = proc.exitCode();

with just one liner

 int exitCode = QProcess::execute(cmdRar, args);

[mbruel]

@JonB
Ok I see. I thought we had to use the overload with separate command and arguments nowadays.
I tried to do a mixed version: the cmd containing also the password bit but letting the rest in the arg list and the process was failing. I didn't try to put everything in the command line...

About the one liner, good to know but in my real app I need a QProcess as I'm connecting its standard outputs and errors ;)

Thanks again for your help!

[aha_1980]

@mbruel

Note that proc.waitForFinished(-1); blocks your application, if you use the signals anyway it might be a good idea to connect the finished signal too and let QProcess run in the background - as packing archives may last more than just some milliseconds.

Regards

[mbruel]

@aha_1980
Well it seems waitForFinished doesn't completely block the main thread as I still manage to receive the standard ready read signals in the main thread. That's true, it's kind of surprising...
I've then cheated a little and force processEvents and my GUI (which is also in the same main thread) is updated kind of in real time with the outputs.
Any idea why this is working?

Here is my code, is it bad practice?

int NgPost::compressFiles(const QString &cmdRar,
                          const QString &cmdPar2,
                          const QString &tmpFolder,
                          const QString &archiveName,
                          const QStringList &files,
                          const QString &pass,
                          int redundancy,
                          const QString &volSize,
                          const QString &compressLevel)
{
    _extProc = new QProcess(this);
    connect(_extProc, &QProcess::readyReadStandardOutput, this, &NgPost::onExtProcReadyReadStandardOutput);
    connect(_extProc, &QProcess::readyReadStandardOutput, this, &NgPost::onExtProcReadyReadStandardOutput);

    // 1.: create archive temporary folder
    QString archiveTmpFolder = QString("%1/%2").arg(tmpFolder).arg(archiveName);
    _compressDir = new QDir(archiveTmpFolder);
...

    // 2.: create rar args (rar a -v50m -ed -ep1 -m0 -hp"$PASS" "$TMP_FOLDER/$RAR_NAME.rar" "${FILES[@]}")
    QStringList args = {"a", "-idp", "-ep1", compressLevel, QString("%1/%2.rar").arg(archiveTmpFolder).arg(archiveName)};
    if (!pass.isEmpty())
        args << QString("-hp%1").arg(pass);
    if (!volSize.isEmpty())
        args << QString("-v%1").arg(volSize);
    args << files;

    // 3.: launch rar
        _log("Compressing files...\n");
    _extProc->start(cmdRar, args);
    _extProc->waitForFinished(-1);
    int exitCode = _extProc->exitCode();
    if (_debug)
        _log(QString("rar exit code: %1\n").arg(exitCode));

...

    // 5.: free the resources
    delete _extProc;
    _extProc = nullptr;
    if (exitCode != 0)
    {
        _error(QString("Error compressing: %1").arg(exitCode));
        delete _compressDir;
        _compressDir = nullptr;
    }
    else
        _log("Ready to post!");

    return exitCode;
}

void NgPost::onExtProcReadyReadStandardOutput()
{
     _log("*");
    qApp->processEvents();
}

void NgPost::onExtProcReadyReadStandardError()
{
    _error(_extProc->readAllStandardError());
    qApp->processEvents();
}

[aha_1980]

@mbruel said in QProcess argument encoding issue...?:

Well it seems waitForFinished doesn't completely block the main thread

It does, if it is run in the main thread.

as I still manage to receive the standard ready read signals in the main thread. That's true, it's kind of surprising...

You will simply get the signals afterwards, which means you have no live update of longer running processes.

Here is my code, is it bad practice?

Depends on your needs, as said if the process runs longer than e.g. 50 ms, I'd always go for asynchronous.

Regards

[mbruel]

@aha_1980 said in QProcess argument encoding issue...?:

You will simply get the signals afterwards, which means you have no live update of longer running processes.

Well look at this small video of my app doing a heavy compression / par2.
My HMI is updated maybe not in real time but definitely during the waitForFinished.
However, my HMI and the NgPost objects both live in the main thread.
Any idea why it is working?

I do need to wait anyway for completion of the QProcess so I'd have to use a QWaitCondition if not directly the waitForFinished of the QProcess... as I can't start the posting tasks until both the compression and the par2 generation are not done.
What you think?

Edit: how/why would you use the QProcess finished signal instead?

[JonB]

@mbruel
I think you're saying you're using signals like QProcess::readyReadStandardOutput() etc. at the same time as QProcess::waitForFinished(). I would not have mixed the signals with the synchronous call. Plus you're putting in QProcess::processEvents(). It's all getting a bit complex.

You're supposed to use the QProcess::finished() signal. No waiting, no processing events, no QWaitCondition. Just QProcess::start(), your UI continues to run, and you proceed with your logic after the compression has completed from QProcess::finished() slot.

[mbruel]

@JonB
Hum I see...
Well I'm receiving the QProcess::readyReadStandardOutput signals while I'm on the QProcess::waitForFinished. That was a bit unexpected but I noticed it was working straight away on my first draft.
I've added the QCoreApplication::processEvents to help populate the signals that update the HMI as I think it was lagging.

I kind of see what you mean with a non blocking implementation that would rely on the QProcess::finished. This means I need a dedicated slot in my main worker (not the HMI) that would then trigger the following tasks.

Well those compression processes have been added as a new feature, it was much faster/easier to keep the existing sequential logic as anyway I don't want to do anything during those QProcess.

I may think about doing a more Qt spirit implementation when I'll have more time.

I'm still quite interested to understand why my current implementation is working. How come I can receive a readdyReadStandardOutput during a waitForFinished? Hum probably because it's a DirectConnection? So that the QProcess that executes them.