kantlivelong / OctoPrint-GCodeSystemCommands

Define G-Code commands that execute local system commands.
GNU Affero General Public License v3.0
37 stars 20 forks source link

Added ability to specify optional parameters #9

Closed aplocher closed 6 years ago

aplocher commented 6 years ago

Added ability to specify optional parameters for the custom GCode that will be passed to the shell script

e.g.

GCode "OCTO100 12 345" Will call "myshellscript.sh 12 345"

If no args are specified, behavior remains the same.

kantlivelong commented 6 years ago

Hi @aplocher,

Thanks for the submission. I've reviewed the PR and I'm not entirely sure how I feel about args being passed from the gcode command itself without some kind of check.

Would requiring a regex match make sense here? Even if the user decides on .* it will at least require the user to accept that risk.

Also you can parse the args with the shlex.split().

aplocher commented 6 years ago

Hmm yeah I was thinking about that too. Sanitize the args to prevent a second command from being injected. I read up on the shlex.split stuff, and using that with subprocess.call seems to be safe.

It seems like these two calls are almost identical and allow the second command to execute: Ex 1 - bad os.system('ls -lhat /; cat /etc/passwd')

Ex 2 - bad subprocess.call('ls -lhat /; cat /etc/passwd', shell=True)

However without the shell=True, and using shlex.split, this will not allow the second command to run:

Ex 3 - good subprocess.call(shlex.split('ls -lhat /; cat /etc/passwd'))

os.system is what is currently in the code. Do you recommend I update that to use the 3rd example

My experience with Python is... minimal, so please excuse my ignorance :) Although for what it's worth, I have a fair amount of shell scripting / programming experience elsewhere.

Thanks

kantlivelong commented 6 years ago

I agree with using subprocess and option 3 however I would suggest using Popen with shell and poll().

Example

kantlivelong commented 6 years ago

Well this is slightly embarrassing. I forgot I already implemented the use of subprocess in the devel branch. You may want to check it out and make your changes against that before updating this PR :smile:

https://github.com/kantlivelong/OctoPrint-GCodeSystemCommands/commits/devel

aplocher commented 6 years ago

No prob. Will do

aplocher commented 6 years ago

Closing this pull request in favor of #10

maurobertorotta commented 4 years ago

OCTO100 12

Hello !

Can you help me about your pull-request GCodeSystemCommands about the ability to specify optional parameters ?

I have undestand that is possible to send a command like OCTO100 50 (for example to send a number to Pan) but in Command Definition of OCTO100 how I can catch the parameter value ?

I have try with : uvcdynctrl -s 'Pan (relative)' %s ... but don't go ...

Sorry for the silly question but I am not a super-guru with this kind of syntax ... ;-)