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

fix: Error passing arguments to chblade-exec-os. #708

Merged
merged 1 commit into from
May 7, 2022

Conversation

Super-long
Copy link
Contributor

@Super-long Super-long commented May 6, 2022

Signed-off-by: Super-long 0x4f4f4f4f@gmail.com

Describe what this PR does / why we need it

When the following statement is executed, the data in /tmp/abc will be truncated:

  1. blade create file append --filepath=/tmp/abc --content="HELL WORLD"
  2. in file /tmp/abc, there is only HELL in it, expected "HELL WORLD"

The cause of the problem is that strings.Split in chaosblade/exec/os/executor.go will truncate "HELL WORLD'" to "HELL" and "WORLD", that is, there are two data in argsArray, then chaosblade-exec- os will incorrectly parse --content as "HELL".

We need to make "HELL WORLD" parse to one item in argsArray.

This bug will make the parsing of all parameters with spaces fail, such as filename, content, etc.

At the same time, I found that #46 has something to do with this bug. In chaos-exec-os, the uid failed to be resolved.

Does this pull request fix one issue?

Fixes #707 and part of #46

Describe how you did it

I made all flags resolve to one item in argsArray and fixed the uid bug.
Now when executing os_exec.CommandContext the argsArray looks like this:

[]string len: 6, cap: 6, [
        "create",
        "file",
        "append",
        "--filepath=/tmp/abc",
        "--content=HELL WORLD",
        "--uid=b4687bde6306a153",

it used to be like this:

create
file
append
--content=HELL
WORLD
--filepath=/tmp/abc
uid=b4687bde6306a153

Describe how to verify it

Executing statements in #707 is now expected behavior.

Special notes for reviews

On the question of whether to add an equals sign on line 64. I try to modify the statement to 'flags = fmt.Sprintf("--%s %s", k, v)', after executing 'dlv exec ./chaos_os -- 'create' 'file' 'append' '--filepath /tmp/abc' '--content HELL WORLD' '-uid 54c63e1814e9c126' get the following results:
'''

main.main() /root/go/src/chaosblade/target/cache/chaosblade-exec-os/main.go:108 (PC: 0x8c5be8)
103: actionFlags := make(map[string]string, len(flagsx))
104: for k, v := range flagsValues {
105: actionFlags[k] = *v
106: }
107: return actionFlags
=> 108: }(),
109: }
110: fmt.Println("5")
111:
112: ctx := context.Background()
113: if mode != spec.Create && mode != spec.Destroy {
(dlv)
befor fun
flag provided but not defined: -filepath /tmp/abc
Usage of /root/go/src/chaosblade/target/chaosblade-1.6.0/bin/chaos_os:
'''

Copy link
Contributor

@EvanSung EvanSung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove flags variable or just define it using short variable declaration in for-loop which is better way.

Signed-off-by: Super-long <0x4f4f4f4f@gmail.com>
@tiny-x
Copy link
Member

tiny-x commented May 7, 2022

Thank you for your contribution

@tiny-x tiny-x merged commit 976a25d into chaosblade-io:master May 7, 2022
@tiny-x tiny-x added the bug label May 7, 2022
@tiny-x tiny-x added this to the v1.6.1 milestone Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

file append experiment encounter bug when parameter --content has space in its value
3 participants