Created attachment 35290 [details] Make creation of command line parameters from GET parameters optional The CGI RFC says, that the server SHOULD create command line arguments from certain GET parameters. https://tools.ietf.org/html/rfc3875#section-4.4 4.4. The Script Command Line I don't like this, because I think, this can be a security risk in certain cases. I suggest to disable this feature by default, or at least allow to disable it by configuration. The proposed patch makes this feature configurable. The line private boolean enableCmdLineArguments = false; makes the feature disabled by default. Putting "= true" would make it enabled by default.
Why I suggest to disable this feature by default: 1) I never saw a servlet that uses this feature 2) I suppose, this feature comes from the beginnings of the internet, when people wanted to run some system command by clicking on a link, and the term "security" was not yet used in computer science :-)
Created attachment 35293 [details] Make evaluation of enableCmdLineArguments in outer if. In fact the evaluation of "boolean enableCmdLineArguments" can happen in the outer if block.
I like the idea of locking this down, but requiring a rebuild-from-source to change the setting isn't acceptable. Can you add a new <init-param> to control this behavior? +1 for making it DISABLED by default (i.e. DO NOT CREATE COMMAND-LINE ARGUMENTS by default).
I don't understand... My patch already uses <init-param>. The patch as it is makes the feature disabled by default.
(In reply to jm009 from comment #4) > I don't understand... > My patch already uses <init-param>. Sorry, you are correct. I did not read the patch closely enough.
I've applied the patch but I do have some feedback for future patches that will make applying your patches quicker and simpler for committers: - Enable checkstyle and ensure the build is clean after your changes - You'll want to configure your IDE to insert 4 spaces rather than a tab - Patches that add configuration options should also include appropriate additions to the documentation To avoid breakage for existing users that may depend on this functionality, it is only disabled by default in 9.0.x onwards.
Thank you for the feedback and for applying the patch.
np and thanks for the patch - just realised I forgot to say that before.