[Geoserver-devel] [10417] branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java: The REAL fix for the request logging filter.

GeoServer GeoServer Developer
1

Hi David,

I guess I misunderstood our last conversation, I did not think any commits were going to occur on 1.7.x... and just for 1.7.1. Regardless, is there a jira issue describing this issue? I don't see one scheduled against 1.7.0. Or a relevant mailing list thread?

Sorry to be a nag, I just want to know why we are breaking the freeze and don't remember any conversation about actual bug.

-Justin

dwinslow@anonymised.com wrote:

Revision
    10417 <http://fisheye.codehaus.org/changelog/geoserver/?cs=10417&gt;
Author
    dwinslow
Date
    2008-10-16 20:13:18 -0500 (Thu, 16 Oct 2008)

      Log Message

The REAL fix for the request logging filter.

      Modified Paths

    * branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java
      <#branches17xgeoserverwebsrcmainjavaorggeoserverfiltersBufferedRequestStreamjava>

      Diff

        Modified:
        branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java
        (10416 => 10417)

--- branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java 2008-10-16 23:59:44 UTC (rev 10416)
+++ branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java 2008-10-17 01:13:18 UTC (rev 10417)
@@ -1,23 +1,22 @@
package org.geoserver.filters;
  import javax.servlet.ServletInputStream;
+import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.io.IOException;
-import java.io.StringReader;
-import java.io.Reader;
  /**
  * Wrap a String up as a ServletInputStream so we can read it multiple times.
  * @author David Winslow <dwinslow@anonymised.com>
  */
public class BufferedRequestStream extends ServletInputStream{
- Reader myReader;
+ InputStream myStream;
      public BufferedRequestStream(String buff) throws IOException {
- myReader = new StringReader(buff);
- myReader.mark(16);
- myReader.read();
- myReader.reset();
+ myStream = new ByteArrayInputStream(buff.getBytes());
+ myStream.mark(16);
+ myStream.read();
+ myStream.reset();
     }
      public int readLine(byte b, int off, int len) throws IOException{
@@ -26,7 +25,7 @@
         int end = off + len;
          while (index < end && - (read = myReader.read()) != -1){
+ (read = myStream.read()) != -1){
             b[index] = (byte)read; index++;
             if (((char)read)== '\n'){
@@ -38,6 +37,10 @@
     }
      public int read() throws IOException{
- return myReader.read();
+ return myStream.read();
     }
+
+ public int available() throws IOException {
+ return myStream.available();
+ }
}

------------------------------------------------------------------------

To unsubscribe from this list please visit:

http://xircles.codehaus.org/manage_email

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

2

Here's the bug:
http://jira.codehaus.org/browse/GEOS-2281

What I did was exactly what I said I would do; roll back the .available() fix (which had been committed on 1.7.x already) and modify the request logging filter to avoid triggering the problem. This bug is pretty minor (it only shows up when POST body logging is enabled, so there's an easy workaround) so waiting for 1.7.1 is fine.

-David

Justin Deoliveira wrote:

Hi David,

I guess I misunderstood our last conversation, I did not think any commits were going to occur on 1.7.x... and just for 1.7.1. Regardless, is there a jira issue describing this issue? I don't see one scheduled against 1.7.0. Or a relevant mailing list thread?

Sorry to be a nag, I just want to know why we are breaking the freeze and don't remember any conversation about actual bug.

-Justin

dwinslow@anonymised.com wrote:

Revision
    10417 <http://fisheye.codehaus.org/changelog/geoserver/?cs=10417&gt;
Author
    dwinslow
Date
    2008-10-16 20:13:18 -0500 (Thu, 16 Oct 2008)

      Log Message

The REAL fix for the request logging filter.

      Modified Paths

    * branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java

      <#branches17xgeoserverwebsrcmainjavaorggeoserverfiltersBufferedRequestStreamjava>

      Diff

        Modified:
        branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java

        (10416 => 10417)

--- branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java 2008-10-16 23:59:44 UTC (rev 10416)
+++ branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java 2008-10-17 01:13:18 UTC (rev 10417)
@@ -1,23 +1,22 @@
package org.geoserver.filters;
  import javax.servlet.ServletInputStream;
+import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.io.IOException;
-import java.io.StringReader;
-import java.io.Reader;
  /**
  * Wrap a String up as a ServletInputStream so we can read it multiple times.
  * @author David Winslow <dwinslow@anonymised.com>
  */
public class BufferedRequestStream extends ServletInputStream{
- Reader myReader;
+ InputStream myStream;
      public BufferedRequestStream(String buff) throws IOException {
- myReader = new StringReader(buff);
- myReader.mark(16);
- myReader.read();
- myReader.reset();
+ myStream = new ByteArrayInputStream(buff.getBytes());
+ myStream.mark(16);
+ myStream.read();
+ myStream.reset();
     }
      public int readLine(byte b, int off, int len) throws IOException{
@@ -26,7 +25,7 @@
         int end = off + len;
          while (index < end && - (read = myReader.read()) != -1){
+ (read = myStream.read()) != -1){
             b[index] = (byte)read; index++;
             if (((char)read)== '\n'){
@@ -38,6 +37,10 @@
     }
      public int read() throws IOException{
- return myReader.read();
+ return myStream.read();
     }
+
+ public int available() throws IOException {
+ return myStream.available();
+ }
}

------------------------------------------------------------------------

To unsubscribe from this list please visit:

http://xircles.codehaus.org/manage_email

3

David Winslow wrote:

Here's the bug:
http://jira.codehaus.org/browse/GEOS-2281

Ok, its not scheduled against a version...

What I did was exactly what I said I would do;

The last message I got from you stated that you would "revert on 1.7.x", which i assumed to mean no commits would be taking place. When in reality you just replaced a commit to one core class with a commit to another. I apologize for missing this in your original mail.

  roll back the

.available() fix (which had been committed on 1.7.x already) and modify the request logging filter to avoid triggering the problem. This bug is pretty minor (it only shows up when POST body logging is enabled, so there's an easy workaround) so waiting for 1.7.1 is fine.

In all honesty I would prefer kicking the bug back. Our "informal" freeze breaking policy has been only to commit changes for serious regressions and bugs, like the recent issue with leaking postgis connections.

Perhaps this bug is serious enough? Not sure. Andrea, what do you think?

-David

Justin Deoliveira wrote:

Hi David,

I guess I misunderstood our last conversation, I did not think any commits were going to occur on 1.7.x... and just for 1.7.1. Regardless, is there a jira issue describing this issue? I don't see one scheduled against 1.7.0. Or a relevant mailing list thread?

Sorry to be a nag, I just want to know why we are breaking the freeze and don't remember any conversation about actual bug.

-Justin

dwinslow@anonymised.com wrote:

Revision
    10417 <http://fisheye.codehaus.org/changelog/geoserver/?cs=10417&gt;
Author
    dwinslow
Date
    2008-10-16 20:13:18 -0500 (Thu, 16 Oct 2008)

      Log Message

The REAL fix for the request logging filter.

      Modified Paths

    * branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java

      <#branches17xgeoserverwebsrcmainjavaorggeoserverfiltersBufferedRequestStreamjava>

      Diff

        Modified:
        branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java

        (10416 => 10417)

--- branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java 2008-10-16 23:59:44 UTC (rev 10416)
+++ branches/1.7.x/geoserver/web/src/main/java/org/geoserver/filters/BufferedRequestStream.java 2008-10-17 01:13:18 UTC (rev 10417)
@@ -1,23 +1,22 @@
package org.geoserver.filters;
  import javax.servlet.ServletInputStream;
+import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.io.IOException;
-import java.io.StringReader;
-import java.io.Reader;
  /**
  * Wrap a String up as a ServletInputStream so we can read it multiple times.
  * @author David Winslow <dwinslow@anonymised.com>
  */
public class BufferedRequestStream extends ServletInputStream{
- Reader myReader;
+ InputStream myStream;
      public BufferedRequestStream(String buff) throws IOException {
- myReader = new StringReader(buff);
- myReader.mark(16);
- myReader.read();
- myReader.reset();
+ myStream = new ByteArrayInputStream(buff.getBytes());
+ myStream.mark(16);
+ myStream.read();
+ myStream.reset();
     }
      public int readLine(byte b, int off, int len) throws IOException{
@@ -26,7 +25,7 @@
         int end = off + len;
          while (index < end && - (read = myReader.read()) != -1){
+ (read = myStream.read()) != -1){
             b[index] = (byte)read; index++;
             if (((char)read)== '\n'){
@@ -38,6 +37,10 @@
     }
      public int read() throws IOException{
- return myReader.read();
+ return myStream.read();
     }
+
+ public int available() throws IOException {
+ return myStream.available();
+ }
}

------------------------------------------------------------------------

To unsubscribe from this list please visit:

http://xircles.codehaus.org/manage_email

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

4

Justin Deoliveira ha scritto:

David Winslow wrote:

Here's the bug:
http://jira.codehaus.org/browse/GEOS-2281

Ok, its not scheduled against a version...

What I did was exactly what I said I would do;

The last message I got from you stated that you would "revert on 1.7.x", which i assumed to mean no commits would be taking place. When in reality you just replaced a commit to one core class with a commit to another. I apologize for missing this in your original mail.

  roll back the

.available() fix (which had been committed on 1.7.x already) and modify the request logging filter to avoid triggering the problem. This bug is pretty minor (it only shows up when POST body logging is enabled, so there's an easy workaround) so waiting for 1.7.1 is fine.

In all honesty I would prefer kicking the bug back. Our "informal" freeze breaking policy has been only to commit changes for serious regressions and bugs, like the recent issue with leaking postgis connections.

Perhaps this bug is serious enough? Not sure. Andrea, what do you think?

No need to touch 1.7.x right now, the bug occurs only on trunk afaik
Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.