Skip to content

Commit

Permalink
Update ExecutionCommand to make current and previous state private to…
Browse files Browse the repository at this point in the history 
… prevent direct modification

- Helper functions are now provided to check for common states. The currentState and previousState must only be modified via setState()
- Some errCode values are also provided to prevent hardcoded value usage.
- The stdout, stderr and arguments will now be truncated for logcat to honour its LOGGER_ENTRY_MAX_LEN limits
  • Loading branch information
@agnostic-apollo
agnostic-apollo committed Mar 25, 2021
1 parent 922f4f4 commit 7ca20fd
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 22 deletions.
8 changes: 4 additions & 4 deletions app/src/main/java/com/termux/app/RunCommandService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public int onStartCommand(Intent intent, int flags, int startId) {
// If invalid action passed, then just return
if (!RUN_COMMAND_SERVICE.ACTION_RUN_COMMAND.equals(intent.getAction())) {
errmsg = this.getString(R.string.error_run_command_service_invalid_intent_action, intent.getAction());
executionCommand.setStateFailed(1, errmsg, null);
executionCommand.setStateFailed(ExecutionCommand.RESULT_CODE_FAILED, errmsg, null);
PluginUtils.processPluginExecutionCommandError(this, LOG_TAG, executionCommand);
return Service.START_NOT_STICKY;
}
Expand All @@ -213,7 +213,7 @@ public int onStartCommand(Intent intent, int flags, int startId) {
// If "allow-external-apps" property to not set to "true", then just return
errmsg = PluginUtils.checkIfRunCommandServiceAllowExternalAppsPolicyIsViolated(this);
if (errmsg != null) {
executionCommand.setStateFailed(1, errmsg, null);
executionCommand.setStateFailed(ExecutionCommand.RESULT_CODE_FAILED, errmsg, null);
PluginUtils.processPluginExecutionCommandError(this, LOG_TAG, executionCommand);
return Service.START_NOT_STICKY;
}
Expand All @@ -228,7 +228,7 @@ public int onStartCommand(Intent intent, int flags, int startId) {
false, false);
if (errmsg != null) {
errmsg += "\n" + this.getString(R.string.msg_executable_absolute_path, executionCommand.executable);
executionCommand.setStateFailed(1, errmsg, null);
executionCommand.setStateFailed(ExecutionCommand.RESULT_CODE_FAILED, errmsg, null);
PluginUtils.processPluginExecutionCommandError(this, LOG_TAG, executionCommand);
return Service.START_NOT_STICKY;
}
Expand All @@ -249,7 +249,7 @@ public int onStartCommand(Intent intent, int flags, int startId) {
true);
if (errmsg != null) {
errmsg += "\n" + this.getString(R.string.msg_working_directory_absolute_path, executionCommand.workingDirectory);
executionCommand.setStateFailed(1, errmsg, null);
executionCommand.setStateFailed(ExecutionCommand.RESULT_CODE_FAILED, errmsg, null);
PluginUtils.processPluginExecutionCommandError(this, LOG_TAG, executionCommand);
return Service.START_NOT_STICKY;
}
Expand Down
66 changes: 48 additions & 18 deletions app/src/main/java/com/termux/models/ExecutionCommand.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.termux.models;

import android.app.Activity;
import android.app.PendingIntent;
import android.net.Uri;

Expand Down Expand Up @@ -49,14 +50,20 @@ public int getValue() {

}

// Define errCode values
// TODO: Define custom values for different cases
public final static int RESULT_CODE_OK = Activity.RESULT_OK;
public final static int RESULT_CODE_OK_MINOR_FAILURES = Activity.RESULT_FIRST_USER;
public final static int RESULT_CODE_FAILED = Activity.RESULT_FIRST_USER + 1;

/** The optional unique id for the {@link ExecutionCommand}. */
public Integer id;


/** The current state of the {@link ExecutionCommand}. */
public ExecutionState currentState = ExecutionState.PRE_EXECUTION;
private ExecutionState currentState = ExecutionState.PRE_EXECUTION;
/** The previous state of the {@link ExecutionCommand}. */
public ExecutionState previousState = ExecutionState.PRE_EXECUTION;
private ExecutionState previousState = ExecutionState.PRE_EXECUTION;


/** The executable for the {@link ExecutionCommand}. */
Expand Down Expand Up @@ -110,7 +117,7 @@ public int getValue() {


/** The internal error code of {@link ExecutionCommand}. */
public Integer errCode;
public Integer errCode = RESULT_CODE_OK;
/** The internal error message of {@link ExecutionCommand}. */
public String errmsg;
/** The internal exceptions of {@link ExecutionCommand}. */
Expand All @@ -137,7 +144,7 @@ public ExecutionCommand(Integer id, String executable, String[] arguments, Strin
@NonNull
@Override
public String toString() {
if(currentState.getValue() < ExecutionState.EXECUTED.getValue())
if(!hasExecuted())
return getExecutionInputLogString(this, true);
else {
return getExecutionOutputLogString(this, true);
Expand All @@ -156,8 +163,7 @@ public static String getExecutionInputLogString(ExecutionCommand executionComman

StringBuilder logString = new StringBuilder();

logString.append(executionCommand.getIdLogString());
logString.append(executionCommand.getCommandLabelLogString()).append(":");
logString.append(executionCommand.getCommandIdAndLabelLogString()).append(":");

if(executionCommand.previousState != ExecutionState.PRE_EXECUTION)
logString.append("\n").append(executionCommand.getPreviousStateLogString());
Expand Down Expand Up @@ -194,8 +200,7 @@ public static String getExecutionOutputLogString(ExecutionCommand executionComma

StringBuilder logString = new StringBuilder();

logString.append(executionCommand.getIdLogString());
logString.append(executionCommand.getCommandLabelLogString()).append(":");
logString.append(executionCommand.getCommandIdAndLabelLogString()).append(":");

logString.append("\n").append(executionCommand.getPreviousStateLogString());
logString.append("\n").append(executionCommand.getCurrentStateLogString());
Expand All @@ -219,7 +224,7 @@ public static String getExecutionOutputLogString(ExecutionCommand executionComma
public static String getExecutionErrLogString(ExecutionCommand executionCommand, boolean ignoreNull) {
StringBuilder logString = new StringBuilder();

if(!ignoreNull || (executionCommand.errCode != null && executionCommand.errCode != 0)) {
if(!ignoreNull || (executionCommand.isStateFailed())) {
logString.append("\n").append(executionCommand.getErrCodeLogString());
logString.append("\n").append(executionCommand.getErrmsgLogString());
logString.append("\n").append(executionCommand.geStackTracesLogString());
Expand Down Expand Up @@ -332,6 +337,10 @@ public String getCommandLabelLogString() {
return "Execution Command";
}

public String getCommandIdAndLabelLogString() {
return getIdLogString() + getCommandLabelLogString();
}

public String getExecutableLogString() {
return "Executable: `" + executable + "`";
}
Expand Down Expand Up @@ -380,11 +389,11 @@ public String getPluginAPIHelpLogString() {
}

public String getStdoutLogString() {
return getMultiLineLogStringEntry("Stdout", stdout, "-");
return getMultiLineLogStringEntry("Stdout", TextDataUtils.getTruncatedCommandOutput(stdout, TextDataUtils.LOGGER_ENTRY_SIZE_LIMIT_IN_BYTES / 5, false, false, true), "-");
}

public String getStderrLogString() {
return getMultiLineLogStringEntry("Stderr", stderr, "-");
return getMultiLineLogStringEntry("Stderr", TextDataUtils.getTruncatedCommandOutput(stderr, TextDataUtils.LOGGER_ENTRY_SIZE_LIMIT_IN_BYTES / 5, false, false, true), "-");
}

public String getExitCodeLogString() {
Expand Down Expand Up @@ -464,7 +473,9 @@ public static String getArgumentsLogString(String[] argumentsArray) {
if (argumentsArray != null && argumentsArray.length != 0) {
argumentsString.append("\n```\n");
for (int i = 0; i != argumentsArray.length; i++) {
argumentsString.append(getSingleLineLogStringEntry("Arg " + (i + 1), argumentsArray[i], "-")).append("`\n");
argumentsString.append(getSingleLineLogStringEntry("Arg " + (i + 1),
TextDataUtils.getTruncatedCommandOutput(argumentsArray[i], TextDataUtils.LOGGER_ENTRY_SIZE_LIMIT_IN_BYTES / 5, true, false, true),
"-")).append("`\n");
}
argumentsString.append("```");
} else{
Expand Down Expand Up @@ -495,7 +506,7 @@ public static String getMultiLineLogStringEntry( String label, Object object,Str
public boolean setState(ExecutionState newState) {
// The state transition cannot go back or change if already at {@link ExecutionState#SUCCESS}
if(newState.getValue() < currentState.getValue() || currentState == ExecutionState.SUCCESS) {
Logger.logError("Invalid ExecutionCommand state transition from \"" + currentState.getName() + "\" to " + "\"" + newState.getName() + "\"");
Logger.logError("Invalid "+ getCommandIdAndLabelLogString() + " state transition from \"" + currentState.getName() + "\" to " + "\"" + newState.getName() + "\"");
return false;
}

Expand All @@ -510,15 +521,18 @@ public boolean setState(ExecutionState newState) {
}

public boolean setStateFailed(int errCode, String errmsg, Throwable throwable) {
if(errCode < 1)
return false;
if (errCode > RESULT_CODE_OK) {
this.errCode = errCode;
} else {
Logger.logWarn("Ignoring invalid " + getCommandIdAndLabelLogString() + " errCode value \"" + errCode + "\". Force setting it to RESULT_CODE_FAILED \"" + RESULT_CODE_FAILED + "\"");
this.errCode = RESULT_CODE_FAILED;
}

this.errmsg = errmsg;

if(!setState(ExecutionState.FAILED))
return false;

this.errCode = errCode;
this.errmsg = errmsg;

if(this.throwableList == null)
this.throwableList = new ArrayList<>();

Expand All @@ -528,4 +542,20 @@ public boolean setStateFailed(int errCode, String errmsg, Throwable throwable)
return true;
}

public boolean isStateFailed() {
if(currentState != ExecutionState.FAILED)
return false;

if(errCode <= RESULT_CODE_OK) {
Logger.logWarn("The " + getCommandIdAndLabelLogString() + " has an invalid errCode value \"" + errCode + "\" while having ExecutionState.FAILED state.");
return false;
} else {
return true;
}
}

public boolean hasExecuted() {
return currentState.getValue() >= ExecutionState.EXECUTED.getValue();
}

}

0 comments on commit 7ca20fd

Please sign in to comment.