Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid potentially sharing execute() state #2760

Merged
merged 2 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion plugins/itemsync/tests/itemsynctests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class TestDir final {

QStringList files() const
{
return m_dir.entryList(QDir::AllEntries | QDir::NoDotAndDotDot, QDir::Name);
QStringList files = m_dir.entryList(QDir::AllEntries | QDir::NoDotAndDotDot, QDir::Name);
files.removeOne(QStringLiteral(".copyq_lock"));
return files;
}

FilePtr file(const QString &fileName) const
Expand Down
53 changes: 24 additions & 29 deletions src/scriptable/scriptable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1904,9 +1904,9 @@ QJSValue Scriptable::execute()
{
m_skipArguments = -1;

m_executeStdoutData.clear();
m_executeStdoutLastLine.clear();
m_executeStdoutCallback = QJSValue();
QByteArray executeStdoutData;
QString executeStdoutLastLine;
QJSValue executeStdoutCallback;

// Pass all arguments until null to command. The rest will be sent to stdin.
QStringList args;
Expand All @@ -1917,7 +1917,7 @@ QJSValue Scriptable::execute()
break;

if ( arg.isCallable() )
m_executeStdoutCallback = arg;
executeStdoutCallback = arg;
else
args.append( toString(arg) );
}
Expand All @@ -1926,7 +1926,7 @@ QJSValue Scriptable::execute()
for ( ++i ; i < argumentCount(); ++i ) {
const auto arg = argument(i);
if ( arg.isCallable() )
m_executeStdoutCallback = arg;
executeStdoutCallback = arg;
else
action.setInput( action.input() + makeByteArray(arg) );
}
Expand All @@ -1935,26 +1935,36 @@ QJSValue Scriptable::execute()
action.setReadOutput(true);

connect( &action, &Action::actionOutput,
this, &Scriptable::onExecuteOutput );
this, [&](const QByteArray &output) {
executeStdoutData.append(output);
});
if ( executeStdoutCallback.isCallable() ) {
connect( &action, &Action::actionOutput,
this, [&](const QByteArray &output) {
executeStdoutLastLine.append( getTextData(output) );
auto lines = executeStdoutLastLine.split('\n');
executeStdoutLastLine = lines.takeLast();
if ( !lines.isEmpty() ) {
const auto arg = toScriptValue(lines, m_engine);
call( "executeStdoutCallback", &executeStdoutCallback, {arg} );
}
});
}

if ( !runAction(&action) || action.actionFailed() ) {
return throwError( QStringLiteral("Failed to run command") );
}

if ( m_executeStdoutCallback.isCallable() ) {
const auto arg = toScriptValue(m_executeStdoutLastLine, m_engine);
call( "executeStdoutCallback", &m_executeStdoutCallback, {arg} );
if ( executeStdoutCallback.isCallable() ) {
const auto arg = toScriptValue(executeStdoutLastLine, m_engine);
call( "executeStdoutCallback", &executeStdoutCallback, {arg} );
}

QJSValue actionResult = m_engine->newObject();
actionResult.setProperty( QStringLiteral("stdout"), newByteArray(m_executeStdoutData) );
actionResult.setProperty( QStringLiteral("stdout"), newByteArray(executeStdoutData) );
actionResult.setProperty( QStringLiteral("stderr"), getTextData(action.errorOutput()) );
actionResult.setProperty( QStringLiteral("exit_code"), action.exitCode() );

m_executeStdoutData.clear();
m_executeStdoutLastLine.clear();
m_executeStdoutCallback = QJSValue();

return actionResult;
}

Expand Down Expand Up @@ -2673,21 +2683,6 @@ void Scriptable::collectScriptOverrides()
m_proxy->setScriptOverrides(overrides);
}

void Scriptable::onExecuteOutput(const QByteArray &output)
{
m_executeStdoutData.append(output);

if ( m_executeStdoutCallback.isCallable() ) {
m_executeStdoutLastLine.append( getTextData(output) );
auto lines = m_executeStdoutLastLine.split('\n');
m_executeStdoutLastLine = lines.takeLast();
if ( !lines.isEmpty() ) {
const auto arg = toScriptValue(lines, m_engine);
call( "executeStdoutCallback", &m_executeStdoutCallback, {arg} );
}
}
}

void Scriptable::onMonitorClipboardChanged(const QVariantMap &data, ClipboardOwnership ownership)
{
COPYQ_LOG( QStringLiteral("onMonitorClipboardChanged: %1 %2, owner is \"%3\"")
Expand Down
6 changes: 0 additions & 6 deletions src/scriptable/scriptable.h
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,6 @@ public slots:
void receiveData();

private:
void onExecuteOutput(const QByteArray &output);
void onMonitorClipboardChanged(const QVariantMap &data, ClipboardOwnership ownership);
void onMonitorClipboardUnchanged(const QVariantMap &data);
void onSynchronizeSelection(ClipboardMode sourceMode, uint sourceTextHash, uint targetTextHash);
Expand Down Expand Up @@ -483,11 +482,6 @@ public slots:
Abort m_abort = Abort::None;
int m_skipArguments = 0;

// FIXME: Parameters for execute() shouldn't be global.
QByteArray m_executeStdoutData;
QString m_executeStdoutLastLine;
QJSValue m_executeStdoutCallback;

bool m_displayFunctionsLock = false;

QJSValue m_plugins;
Expand Down
Loading