[java] Potential bugs when using java.lang.ProcessBuilder

Clean Code New rules / language support
1

Hi Java experts,

It’s quite easy to badly use java.lang.ProcessBuilder:

  • processes may block if streams are not correctly processed
  • the stderr stream of the child process may be ignored. That occurred recently in SonarCloud Autoscan feature :frowning:

A great detailed description is available at https://wiki.sei.cmu.edu/confluence/display/java/FIO07-J.+Do+not+let+external+processes+block+on+IO+buffers.

That deserves a new rule to detect the bad consumption of process streams. For example:

  • Process#getInputStream() should be called once before waitFor(). That does not ensure that stream is correctly consumed and false-negatives are still possible, but it’s already a valuable indicator.
  • stderror is lost if Process#getErrorStream() is not “gobbled”. If this method is not called before waitFor(), then the solution is to call ProcessBuilder#redirectErrorStream(true).

++

2

Hi Simon,

The rule looks good, I would just modify as follow:

  • Process#getInputStream() or ProcessBuilder#redirectOutput(...) should be called once before waitFor() . That does not ensure that stream is correctly consumed and false-negatives are still possible, but it’s already a valuable indicator.
  • stderror is lost if Process#getErrorStream() is not “gobbled”. If this method is not called before waitFor() , then the solution is to call ProcessBuilder#redirectErrorStream(true) or ProcessBuilder#redirectError(...).

Note the additional ProcessBuilder#redirectError(...) and ProcessBuilder#redirectOutput(...). They are used for example in Elasticsearch.