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

wbt_laplacian_filter() variant= OptionList argument type needs shell quoting #46

Closed
brownag opened this issue Aug 27, 2021 · 0 comments · Fixed by #79
Closed

wbt_laplacian_filter() variant= OptionList argument type needs shell quoting #46

brownag opened this issue Aug 27, 2021 · 0 comments · Fixed by #79

Comments

@brownag
Copy link
Member

brownag commented Aug 27, 2021

This is a note to make aware of some unexpected behavior I found while testing functions systematically.

I made some notes on this but never posted an issue. Still pondering what the general approach should be for sanitizing user input data and types in the autogenerated methods. I have some tools that I have been working for checking the arg string that might be able to be dropped into all of the generated code.

This is an analogue of #20 where arguments plopped into shell can lead to unexpected results. In this issue the shell does not like the parentheses floating around unquoted.


This wbt_laplacian_filter() behavior shows up as Syntax error: "(" unexpected

Fix: Default variant user argument needs to be wrapped in shQuote(). It will fail with the default value "3x3(1)" . A heavy-handed solution might be to shQuote all "OptionList" parameter types. I say heavy handed because it might not happen all that frequently. At a minimum any default arguments should be quoted so they dont error, but probably it should just quote the argument, which would only affect people who may have come up with their own workaround. Note: WBT should handle extra quotes for arguments fine, it replaces anything that evaluates to " or ' with nothing.

Of the other OptionList cases (n=5) where the variant parameter name is used wbt_laplacian_filter() is the only one that is sensitive to this bug because of the parentheses in the option labels.

Fix (to be done in a general way)

Change

args <- paste(args, paste0("--variant=", variant))

where variant
to

args <- paste(args, paste0("--variant=", shQuote(variant)))

So that
--run=laplacian_filter --input=DEM.tif --output=slope.tif --variant=3x3(1) --clip=0 -v
becomes
--run=laplacian_filter --input=DEM.tif --output=slope.tif --variant='3x3(1)' --clip=0 -v


function_name label parameter_type description
wbt_extract_valleys variant OptionList LQ, JandR, PandD Options include 'LQ' (lower quartile), 'JandR' (Johnston and Rosenfeld), and 'PandD' (Peucker and Douglas); default is 'LQ'.
wbt_laplacian_filter variant OptionList 3x3(1), 3x3(2), 3x3(3), 3x3(4), 5x5(1), 5x5(2) Optional variant value. Options include 3x3(1), 3x3(2), 3x3(3), 3x3(4), 5x5(1), and 5x5(2) (default is 3x3(1)).
wbt_line_detection_filter variant OptionList vertical, horizontal, 45, 135 Optional variant value. Options include 'v' (vertical), 'h' (horizontal), '45', and '135' (default is 'v').
wbt_sobel_filter variant OptionList 3x3, 5x5 Optional variant value. Options include 3x3 and 5x5 (default is 3x3).
wbt_tophat_transform variant OptionList white, black Optional variant value. Options include 'white' and 'black'.


brownag added a commit to brownag/whiteboxR that referenced this issue May 6, 2022
@brownag brownag linked a pull request May 6, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant