On 2011-10-31, Bear Giles wrote:
> I found a few issues (1330) with Fortify. As anyone who's used it knows the
> vast majority of those are of the "but that's what I intended" variety, but
> there were 180 cases of unclosed resource streams and 354 cases of
> potential denial of service.
> In the first case we all write
> InputStream is = get some stream;
> is.read();
> is.close(); // maybe, or maybe we just let it go out of scope.
> but Fortify wants:
> InputStream is = null;
> try {
> is = get some stream;
> is.read();
> } finally {
> if (is != null) {
> is.close(); // could possibly throw a second exception here
> }
Do we have cases of this in the main source tree? I know they are in
the tests but I'd expect the main tree to do just fine.
> In the second case we write something like:
> public int read(byte[] b, int from, int length) throws IOException {
> int read = in.read(b, from, length);
> this.count(read);
> return read;
> but Fortify wants something like
> public int read(byte[] b, int from, int length) throws IOException {
> if (b != null || b.length < length) {
> throw new IllegalArgumentException("buffer must be at least " +
> length + " bytes long.");
> }
> // and probably also
> if (from < 0 || length < 1) {
> throw new IllegalArgumentException("...");
> }
> int read = in.read(b, from, length);
> this.count(read);
> return read;
> which is hard to argue with even though it will take a little longer to
> execute.
I'm with the shool of thought that expects the wrapped stream to deal
with it 8-)
> Is this something worth addressing now or should it wait until 2.0?
Let's do this for the next release. Whether this is 1.4 or 2.0.
Thanks
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
