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 21:03:34 GMT
Quick followup - I know there's also a school of thought that bad
parameters should be passed to the InputStream and let it throw the
exception since it's the method that actually cares about the values.
 Maybe a negative offset will have meaning at some point in the future. So
you can argue that the code would be cleaner if we (explicitly) let the
InputStream method handle bad parameters or it will be cleaner if we
explicitly check for null or short buffers or if we do a mixture.


On Mon, Oct 31, 2011 at 2:59 PM, Bear Giles <bgiles@coyotesong.com> 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
>      }
>   }
>
> 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