Termux:Tasker Plugin Various Improvements#36
Merged
15 commits merged intotermux:masterfrom Dec 6, 2020
Merged
Conversation
…s like normally done on shells like bourne shell
Make `PluginResultsService` a unified class and service to handle sending of plugin commands to the execution service, receiving the results of plugin commands back from the execution service and sending immediate or pending results back to the plugin host.
…er`. - Changed `ResultsService` service name to `PluginResultsService`.
- Moved `getVersionCode()` function to `PluginUtils` class.
… and `TermuxAppUtils` util classes.
…ger` util class. - Added support to store `log_level` persistently in android `QueryPreferences` using `QueryPreferences` util class.
…ory, provided that `allow-external-apps` property is set to `true` by the user in `~/.termux/termux.properties` file. - Added support for working directory. - Adding support for users to set plugin `logcat` log levels to help with debugging through the configuration activity options menu. - Added support to automatically clear `%errmsg`, `%stdout`, `%stderr` and `%result` variables when plugin action is run. - Adding support for showing warnings in configuration activity and returning error messages via `%errmsg` to plugin host app on invalid plugin configuration or missing permissions or access failures. - Updated `FireReceiver` to use `PluginResultsService` for sending commands to execution service and to send error messages to plugin host app if required. - Updated default action timeout value to `10s` regardless of `inTerminal` value. - Removed requirement for `TASKER_DIR` to exist for plugin configuration since absolute paths are allowed. - Fixed `isBundleValid()` function.
- Changed `case` statements to `if` since resource IDs are non-final from gradle plugin version `5`.
- Updated library versions and switched to `AndroidX`.
37b97b0 to
aa13bb4
Compare
Member
Author
|
That was quick. Thanks! |
This was referenced Dec 16, 2020
Closed
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey,
Here are a bunch of improvements for Termux:Tasker. Check the new
README.mdfor more details, some things not detailed here to prevent duplication.json. The argument splitting in code was done here.Without the java escaping, the regex is
([^"]\S*|".+?")\s*.What that means in layman is that:
(non double quote character followed by zero or more non white space characters OR one or more characters surrounded by double quotes) followed by zero or more whitespace charactersBasically string splits on whitespaces unless enclosed in double quotes. But the second part of the OR does not check if double quotes are preceded by a backslash so to ignore them. So if you pass
"some string with \" double quote", i think it will likely become two argumentssome string with \anddouble quoteAfter that it also replaces all double quotes with nothing in line
47. So technically there was no way to pass double quotes from what I saw.I have fixed that by using ArgumentTokenizer which will parse the arguments as normally done on the shell, including surrounding with single quotes for sending literal strings, of course any single quotes inside the string that needs to be passed will have to be escaped.
Since termux can run dangerous commands, there should be better security. I have added
com.termux.permission.RUN_COMMANDto theFireReceiverthat will be required to run ANY termux commands with the plugin from henceforth. TheTaskerapp has requested the permission sincev5.9.3so an update would not be required for it. Other plugin host apps will have to be updated for the plugin to continue working.Added support for absolute paths outside
~/.termux/tasker/directory, provided thatallow-external-appsproperty is set totrueby the user in~/.termux/termux.propertiesfile. The permission combined with the property check provides reasonable security for apps to execute arbitrary commands. This is already done by theRUN_COMMANDintent. The benefit would be that the result of commands can be received by the sender like Tasker as well. Note that this behaviour is a bit different fromRUN_COMMANDintent, it requires both to run any commands, whereas the plugin will only require the permission to run scripts in tasker directory and both to run absolute paths. This will make it easier for users to just grant the permission for scripts and they can optionally not set the property totrueif they don’t want plugin apps to run arbitrary commands. If you want this behaviour changed, let me know.Added support for working directory, specially since absolute paths are now supported. It will be automatically created if inside
~/.Adding support for users to set plugin
logcatlog levels to help with debugging through the configuration activity options menu.The whole app is kinda refactored with safety checks everywhere, notifying the user of errors in every step, like if they are missing a permissions and if they have invalid values set or if
$PREFIXcan't be accessed by the plugin in case oftermux-appnot installed or mixing of install sources. The%errmsgvariable will also be used for reporting errors to tasker while running the plugin action that occurs before command execution.I have added the
PluginResultsServiceclass that is a unified class and service to handle sending of plugin commands to the execution service, receiving the results of plugin commands back from the execution service and sending immediate or pending results back to the plugin host. Thetermux-appshould consider importing the class and using thesendExecuteResultToResultsService()function inBackgroundJobto send exceptions in%errmsgso that plugin host apps can be notified instead of users having to checklogcatand also for sending back the final result in%stdoutetc.Template tasker task and scripts for running the plugin have been added in
templates/.There is a kinda a related bug mentioned by @Grimler91 here which was being caused by
Termux:Taskerplugin sendingTaskerPlugin.Setting.RESULT_CODE_PENDINGback to tasker for commands that had to be run in the terminal session and the tasker action had been set to a timeout of>0. The issue was thatTermuxServicedidn't send any result viaPendingIntentback to the plugin ifEXTRA_EXECUTE_IN_BACKGROUNDwasfalse, so the plugin also never sent any result back to tasker even though tasker was expecting it and hence action timed out. I have fixed that by sendingTaskerPlugin.Setting.RESULT_CODE_OKdirectly from the plugin ifinTerminalistrue. Timeout will also default to10snow regardless ofinTerminalvalue so that%errmsgcan be returned to plugin host app, since otherwise if it were set to0s, plugin host wouldn't wait for any errors and continued the task and user wouldn't know what happened, like ifallow-external-appsis set tofalseand absolute path out~/.termux/tasker/directory is set asExecutable. The users too should update their tasks and use the default10seven withinTerminaltrue. The%errmsg,%stdout,%stderrand%resultvariables will also be cleared automatically whenever the action is run if timeout is greater than0to solve the issue of if multiple actions are run in the same task, then variables from previous action may still be set and get mixed in with current ones.I have updated the code to
AndroidXand made other changes so it can be compiled fortargetSdkVersion29when needed just by incrementing the number, it currently still targets28sincetermux-appisn't ready. Playstore uploads will not be possible sincenov 2deadline has passed and uploads are not planned anyways untiltermux-appis ready. The plugin works fine when targeting sdk29with the currenttermux-apptargeting sdk28and also withandroid-10branch targeting sdk29.If
termux-apptargets sdk28andtermux-taskertargets sdk29on android 10 emulator.The
termux-taskergivesavc: granted { execute }warnings whencanExecute()is called:The
termux-appgives the following warnings on execution:These are likely due to
auditallowrules and just security notifications and hopefully won't get changed to denial messages in future, but who knows. ;)If
termux-app(android-10 branch) andtermux-taskerboth target sdk29on android 11 emulator, the scripts and running bash commands do execute fine, although there are followingavcwarnings, not sure what's causing them. Thelib275.soisbash.Moreover, due to apparent proot usage to run scripts, the script shebangs need to be set to
#!/usr/bin/bashinstead of#!/data/data/com.termux/files/usr/bin/bash, otherwise scripts fail. Is that behaviour likely to stay, since that may break current scripts of users. TheTermux Environmentsection of theREADME.mdwould require updating too. I have set the template scripts shebangs to#!/usr/bin/...just in case.So basically, since no executions are directly done in
termux-tasker, the app should work fine if it targets sdk29. Changes just have to be made on thetermux-appside. Shebangs will cause problems for existing scripts though. Although, pushing an update fortermux-taskerto playstore for bug fixes can be done by targeting sdk29and keepingtermux-appat28, since its working that way, but will requiretermux-appto be installed before the plugin, otherwise due tosharedUserId, sdk29exec restrictions kick in, so that could be troublesome.The plugin version should ideally be incremented to
0.5since that's referenced everywhere in theREADME.mdand templates.unrelated The
termux-appBackgroundJob.BackgroundJob()andTermuxService.createTermSession()functions need to be updated for targeting sdk29if (cwd == null) cwd = TermuxService.HOME_PATH;needs to be changed toif (cwd == null || cwd.isEmpty()) cwd = TermuxService.HOME_PATH;since otherwise if an empty working directory string extra is passed from theRUN_COMMANDintent or the plugin, the following exception is raised due to an empty string being passed toRuntime.getRuntime().exec(progArray, env, new File(cwd));. I have fixed that from the plugin side by not passing an empty directory but this should be fixed fromtermux-appside as well, cause of issues withRUN_COMMANDintent and possibly others.termux-appRunCommandService.parsePath()function needs to be fixed as perFileUtils.getExpandedTermuxPath()function intermux-tasker. The working directory should also be created automatically if inside termux home as pertermux-tasker. There should also be a dedicated Wiki page added forRUN_COMMANDintent so its easier to find for users and devs instead of its brief mention in FAQ, something that shows in search results easily likeRunning termux commands from other apps.