CFE-2986: Added getdir command to cf-net, copies directory and files

[SimonThalvorsen]

Ticket: CFE-2986
Changelog: Title

[larsewi]

It would be nicer if we extended cf-net get to get both files and directories, instead of having two separate sub-commands. Please investigate how rsync does this and try to do it in a similar way here.

[olehermanse]

It would be nicer if we extended cf-net get to get both files and directories, instead of having two separate sub-commands. Please investigate how rsync does this and try to do it in a similar way here.

@larsewi on the other hand, cf-net was written mainly as a protocol testing tool, and the commands mirror their protocol command names. If the GET protocol command in CFEngine network protocol can only GET a file, it might be a bit confusing that cf-net get is something very different.

There is also some advantage to not being too dynamic with this stuff. If you make a CLI that forces the user to specify if they are copying a file or a folder, you can prevent some annoying / confusing situations.

We could for example make a new subcommand, copy;

$ cf-net copy --file promises.cf
Copying file 1.2.3.4:/var/cfengine/masterfiles/promises.cf -> ./promises.cf

$ cf-net copy --file services/
Error: 'services/' does not look like a file, did you mean --directory?

$ cf-net copy --file services/.
Error: 'services/.' does not look like a file, did you mean --directory?

$ cf-net copy --directory services/
Copying directory 1.2.3.4:/var/cfengine/masterfiles/services/ -> ./services/

$ cf-net copy services/
Copying directory 1.2.3.4:/var/cfengine/masterfiles/services/ -> ./services/

$ cf-net copy services
Warning: Ambiguous if 'services' is a file or a directory - consider adding --file or --directory.
Copying directory 1.2.3.4:/var/cfengine/masterfiles/services/ -> ./services/

[larsewi]

Please break this up a little. Maybe a recursive solution would be more elegant given that it is a recursive problem.

[larsewi]

There should be a recursion depth limit to prevent a potential malicious or misconfigured server with symlink loops (or deeply nested dirs) to cause stack overflow.

We should probably stick to a maximum path size for the same reasons above.

[larsewi]

Make sure to check return values of functions. It's OK to do best effort. I.e., we encounter an error but continue if it's safe. But in this case we need to cache the error so that we can later exit with failure. For some errors, like if we fail to create a directory, it does not make sense to try to fetch the files that should be in that directory afterwards.

[larsewi]

It's starting to look good. Just a few more nitpicks. Almost there!

[larsewi]

Use after free. Base points to tmp, and tmp was free'd on line 1229. Above you should probably copy the string to a local buffer char tmp[PATH_MAX];. Then call basename on the stack allocated buffer.

[SimonThalvorsen]

base uses xstrdup on the basename of tmp, so it does not point into the same buffer