Return-Path: X-Original-To: apmail-commons-dev-archive@www.apache.org Delivered-To: apmail-commons-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 5600F9D1C for ; Mon, 31 Oct 2011 21:04:21 +0000 (UTC) Received: (qmail 24008 invoked by uid 500); 31 Oct 2011 21:04:20 -0000 Delivered-To: apmail-commons-dev-archive@commons.apache.org Received: (qmail 23915 invoked by uid 500); 31 Oct 2011 21:04:20 -0000 Mailing-List: contact dev-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Commons Developers List" Delivered-To: mailing list dev@commons.apache.org Received: (qmail 23902 invoked by uid 99); 31 Oct 2011 21:04:20 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 31 Oct 2011 21:04:20 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of bgiles@coyotesong.com designates 209.85.213.43 as permitted sender) Received: from [209.85.213.43] (HELO mail-yw0-f43.google.com) (209.85.213.43) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 31 Oct 2011 21:04:16 +0000 Received: by ywp17 with SMTP id 17so10560088ywp.30 for ; Mon, 31 Oct 2011 14:03:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=coyotesong.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type; bh=jAp9STGTi4pDrqtTiglr+RkV4LPqeQBV8EGeMDwKzK4=; b=HvhMitvC2cYghTSzN46o1olGEqytYKzkiinBo6LLkmo/cnq9vY9xi6THZfCsYLPeHl ccT54P0Ync4Kt+fCGpwhwjfvggpPCevAgzt1OUbFLwFf471Ha0K/H/a6FYyhdNhhWUS8 4PGC12zq75i0IJ0Ae2xG3F9LQabY08ccnJTbE= Received: by 10.42.153.74 with SMTP id l10mr24887267icw.52.1320095035122; Mon, 31 Oct 2011 14:03:55 -0700 (PDT) MIME-Version: 1.0 Received: by 10.42.225.132 with HTTP; Mon, 31 Oct 2011 14:03:34 -0700 (PDT) In-Reply-To: References: <87fwidsnog.fsf@v35516.1blu.de> From: Bear Giles Date: Mon, 31 Oct 2011 15:03:34 -0600 Message-ID: Subject: Re: [VOTE] Release Compress 1.3 based on RC2 To: Commons Developers List Content-Type: multipart/alternative; boundary=90e6ba2121b9a3650b04b09e9351 --90e6ba2121b9a3650b04b09e9351 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 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" variet= y, > 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 =3D get some stream; > is.read(); > is.close(); // maybe, or maybe we just let it go out of scope. > > but Fortify wants: > > InputStream is =3D null; > try { > is =3D get some stream; > is.read(); > } finally { > if (is !=3D 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 =3D 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 !=3D 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 =3D 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 th= e > 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 wro= te: > >> +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=F6rg Schaible > >wrote: >> >> > Stefan Bodewig wrote: >> > >> > > Hi all, >> > > >> > > compared to RC1 Michael Kuss' name has been fixed and he's been adde= d >> as >> > > contributor. A bunch of additional tests increased coverage (still >> some >> > > areas are not covered but coverage is better than it has been for an= y >> > > 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 maven= 2. >> > > >> > > The tag is here (svn revision 1190156): >> > > >> > >> http://svn.apache.org/viewvc/commons/proper/compress/tags/COMPRESS_1.3_R= C2/ >> > > >> > > 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=F6rg >> > >> > >> > >> > --------------------------------------------------------------------- >> > 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://bit.ly/ECvg0 >> Spring Batch in Action: http://bit.ly/bqpbCK >> Blog: http://garygregory.wordpress.com >> Home: http://garygregory.com/ >> Tweet! http://twitter.com/GaryGregory >> > > --90e6ba2121b9a3650b04b09e9351--