If this is a real concern then we can do the following: * roll back the aforementioned change * tweak the logging filter to do a read and a rewind on the stream that it's returning to ensure that available() returns something bigger than zero * increase test coverage and do it this way (I'd argue the 'right' way) for 1.7.1
That said, stream.available() == 0 does not imply that the stream has no contents, just that 0 bytes can be read without blocking. Relying on it to return >0 for non-empty streams will cause problems in some situations such as this one, so ultimately I'd like to see the .available()-less implementation used here. For what it's worth, I haven't seen any issues since making this change; but I also have not made a point of trying out the different edge cases wrt POST/GET parameter mixing. -David Winslow Justin Deoliveira wrote: > Hi David, > > I am a little concerned about this patch. I don't have a concrete -1 > against it but I remember that check handling various cases and an issue > arising when I once tried to take it out as well. > > Andrea might have a better idea of what that issue actually was. But > just a word of caution to proceed carefully. > > It might be good for us to add a few more test cases to the dispatcher > tests testing some of the more obscure cases. Such as simulating input > from a form... or sending both content in the body and parameters as kvp's. > > -Justin > > [EMAIL PROTECTED] wrote: > >> Revision >> 10383 <http://fisheye.codehaus.org/changelog/geoserver/?cs=10383> >> Author >> dwinslow >> Date >> 2008-10-13 17:21:06 -0500 (Mon, 13 Oct 2008) >> >> >> Log Message >> >> Don't use .available() to check whether streams have data in them. Fixes >> GEOS-2281 <http://jira.codehaus.org/secure/ViewIssue.jspa?key=GEOS-2281> >> (log filter killing POST requests when body logging enabled) >> >> >> Modified Paths >> >> * trunk/geoserver/ows/src/main/java/org/geoserver/ows/Dispatcher.java >> <#trunkgeoserverowssrcmainjavaorggeoserverowsDispatcherjava> >> >> >> Diff >> >> >> Modified: >> trunk/geoserver/ows/src/main/java/org/geoserver/ows/Dispatcher.java >> (10382 => 10383) >> >> --- trunk/geoserver/ows/src/main/java/org/geoserver/ows/Dispatcher.java >> 2008-10-13 19:03:31 UTC (rev 10382) >> +++ trunk/geoserver/ows/src/main/java/org/geoserver/ows/Dispatcher.java >> 2008-10-13 22:21:06 UTC (rev 10383) >> @@ -223,17 +223,19 @@ >> //create the kvp map >> parseKVP(request); >> >> - if ( !request.get && httpRequest.getInputStream().available() > 0) { >> - //wrap the input stream in a buffer input stream >> + if ( !request.get ) { // && >> httpRequest.getInputStream().available() > 0) { >> + //wrap the input stream in a buffered input stream >> request.input = reader(httpRequest); >> >> - //mark the input stream, support up to 2KB, TODO: make this >> configuratable >> + //mark the input stream, support up to 2KB, TODO: make this >> configurable >> request.input.mark(2048); >> >> if (logger.isLoggable(Level.FINE)) { >> char[] req = new char[1024]; >> int read = request.input.read(req, 0, 1024); >> - if (read < 1024) { >> + if (read == -1) { >> + request.input = null; >> + } else if (read < 1024) { >> logger.fine("Raw XML request starts with: " + new >> String(req)); >> } else { >> logger.fine("Raw XML request starts with: " + new >> String(req) + "..."); >> >> ------------------------------------------------------------------------ >> >> To unsubscribe from this list please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > > > ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Geoserver-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geoserver-devel
