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

PackageManager problems #84

Closed
janosvitok opened this Issue May 25, 2018 · 2 comments

Comments

2 participants
@janosvitok
Collaborator

janosvitok commented May 25, 2018

Issue 1: all options get space appended

Issue 2: option parameter is joined with the value:

stringBuilder.append(str).append(" ");

WITH_FORWARD_LOCK.getStringRepresentation() -> "-l "
WITH_INSTALLER_PACKAGE_NAME("aaa bbb").getStringRepresentation() -> "-t aaa bbb"

that JadbDevice.buildCmdLine turns to

'-l '
'-t aaa bbb'

while most probably it should be

-l
-t 'aaa bbb'

Issue 3: Bash.quote() in PackageManager.remove() probably causes double quoting.

Issue 4: Bash.quote itself propably does not quote well

Bash.quote("-t 'aaa'") ->"'-t '\'aaa\'''"
Edit: I've learned that the code is correct.
I've create PR #90 with a test for this.

Possible fixes

Easy fix Issue 1 is to add

            stringBuilder.deleteCharAt(stringBuilder.length() -1);

at the end of InstallOption constructor.

Fix for Issue 2 (will fix Issue 1) is to store String[] and just pass unprocessed them from installWithOptions() through install() to executeShell() that will add spaces and quotes anyway.

@vidstige

This comment has been minimized.

Owner

vidstige commented Jun 17, 2018

Thank you for taking your time to report this. Perhaps this can be split into four different issues to make it easier to handle, and tick off one by one? 🤔

If you feel so inclinded, pull requests are warmly welcome. :)

smieras added a commit to smieras/jadb that referenced this issue Sep 4, 2018

vidstige added a commit that referenced this issue Sep 10, 2018

Merge pull request #97 from smieras/master
#84 PackageManager problems: Fixes issue 1 & 2
@vidstige

This comment has been minimized.

Owner

vidstige commented Sep 17, 2018

All but one problem remaining. Moved it to separate issue for clarity - #105.

@vidstige vidstige closed this Sep 17, 2018

@vidstige vidstige referenced this issue Sep 19, 2018

Closed

Double quoting #105

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment