commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bear Giles <bgi...@coyotesong.com>
Subject Re: [VOTE] Release Compress 1.3 based on RC2
Date Mon, 31 Oct 2011 20:59:55 GMT
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
     }
  }

This ensures that the input stream will always be closed. Going out of
scope SHOULD call the finalize method but there's no guarantee that this
will happen and it's possible that the stream was left open.


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.

On the one hand these seem extremely picky. On the other hand this is a
library used by others, not our own code where we have a lot more
flexibility in changing it if things go wrong. (I know, people can get the
source and compile it themselves but it will take time to track down the
problem, identify a solution, etc.)

Is this something worth addressing now or should it wait until 2.0?

Bear Giles


On Sun, Oct 30, 2011 at 7:38 PM, Gary Gregory <garydgregory@gmail.com>wrote:

> +1
>
> Looks good on Oracle Java 1.6.0_29 and 1.7.0_01, Maven 3.0.3 on Windows 7
> 64 bit.
>
> Apache Maven 3.0.3 (r1075438; 2011-02-28 12:31:09-0500)
> Maven home: C:\Java\apache-maven-3.0.3\bin\..
> Java version: 1.7.0_01, vendor: Oracle Corporation
> Java home: C:\Program Files\Java\jdk1.7.0_01\jre
> Default locale: en_US, platform encoding: Cp1252
> OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"
>
> Gary
>
> On Sun, Oct 30, 2011 at 2:41 PM, Jörg Schaible <joerg.schaible@gmx.de
> >wrote:
>
> > Stefan Bodewig wrote:
> >
> > > Hi all,
> > >
> > > compared to RC1 Michael Kuss' name has been fixed and he's been added
> as
> > > contributor.  A bunch of additional tests increased coverage (still
> some
> > > areas are not covered but coverage is better than it has been for any
> > > prior release).  A few plugins have been upgraded.
> > >
> > >   Compress 1.3 RC2 is available for review here:
> > >     http://people.apache.org/~bodewig/compress-1.3-RC2/
> > >
> > >   Maven artifacts are here:
> > >
> >
> >
> https://repository.apache.org/content/repositories/orgapachecommons-111/org/apache/commons/commons-
> > compress/1.3/
> > >
> > >   Details of changes since 1.1 are in the release notes:
> > >
> http://people.apache.org/~bodewig/compress-1.3-RC2/RELEASE-NOTES.txt
> > >     http://people.apache.org/~bodewig/compress-1.3-RC2/site/changes-
> > report.html
> > >
> > >   I have tested this with Oracle JDK 1.5 and OpenJDK 1.6 using maven2.
> > >
> > >   The tag is here (svn revision 1190156):
> > >
> >
> http://svn.apache.org/viewvc/commons/proper/compress/tags/COMPRESS_1.3_RC2/
> > >
> > >   Site:
> > >     http://people.apache.org/~bodewig/compress-1.3-RC2/site/
> > >
> > >   Clirr Report (compared to 1.2):
> > >     http://people.apache.org/~bodewig/compress-1.3-RC2/site/clirr-
> > report.html
> > >
> > >   RAT Report:
> > >     http://people.apache.org/~bodewig/compress-1.3-RC2/site/rat-
> > report.html
> > >
> > >   Votes, please.  This vote will close in 96 hours, 0500 GMT
> 01-November
> > >   2011
> > >
> > >   [ ] +1 Release these artifacts
> > >   [ ] +0 OK, but...
> > >   [ ] -0 OK, but really should fix...
> > >   [ ] -1 I oppose this release because...
> >
> > +1
> >
> > Build from source with my complete compiler zoo.
> >
> > - Jörg
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> JUnit in Action, 2nd Ed: <http://goog_1249600977>http://bit.ly/ECvg0
> Spring Batch in Action: <http://s.apache.org/HOq>http://bit.ly/bqpbCK
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message