Alan Bateman wrote:
Michael McMahon wrote:
We could add a sentence to the existing @throws clause for IAE.
So it would say "If |command| is empty, or on some platforms, may be
thrown if command contains illegal characters".
I'm reluctant to be more precise than that, since as far as I know,
there doesn't appear to be
any illegal characters for filenames in Unix.
I agree that you can't be too specific. I'm not sure if "illegal
characters" should be in the wording as it begs the question as to
which characters are legal. An alternative to trying to come up with
some wording is to just specify the long standing behavior which seems
to be that IOException is thrown if an I/O errors or the command line
is malformed. I see David's reply pointing out that there are similar
issues with ProcessBuilder. That is only specified to throw
IndexOutOfBoundException if the command line is empty so it would be
good to check those cases too.
-Alan.
I went through the rest of the changes.
In src/solaris/classes/java/lang/ProcessImpl.java then the permission
check needs to happen before the streams are redirected as otherwise it
would allow an attacker to truncate arbitrary files. I think the
alignment is a bit out at L132-142.
In src/windows/classes/java/lang/ProcessImpl.java L211-216 is using
toLowerCase (which is locale specific) and is assuming that prog and
lprog will be the same length.
The algorithm in getProgramPath seems okay although I think it can be
cleaned up a bit (coding style is inconsistent with the rest of the file
for example, is the StringBuilder actually needed). Minor comment is
that "fileHasNoDot" is an odd method name, maybe hasDot would be nicer.
There's space between File and "(" at line 323.
A general comment is that what would it take to move this code away from
using StringTokenizer? I realize Runtime.exec has javadoc to say that it
uses StringTokenizer and I just wonder if that can be re-visited.
-Alan.
