David Winslow wrote: > 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 +1. I would say go ahead with the change on trunk. And lets write some test cases for the dispatcher testing some of the corner cases. And when we feel more confident we can commit on 1.7.x 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. > I agree, the check is not 100% robust, but it was put in to fix a particular use case so I fear changing it will introduce a regression. But I realize that because I can't remember what the case is I am not making a very strong case.
------------------------------------------------------------------------- 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
