if nothing happens with the capture - no header or status set - should report as handled

- weird yet works with Tomcat as before and serves static content with Jetty right again
- make sure when asked and reported isHandled as truthy it stays (due the container)
- also do some internal cleanup - whole output swalowing should get a review ...

fixes #175

Author: kares

src/main/java/org/jruby/rack/servlet/ResponseCapture.java

@@ -8,7 +8,6 @@
 package org.jruby.rack.servlet;
 
 import java.io.IOException;
-import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.lang.reflect.InvocationTargetException;
@@ -65,11 +64,11 @@ public boolean isStatusSet() {
      *
      * @param status the new HTTP status
      * @param error whether the status comes from a {@code sendError}
-     * @see #isHandled()
+     * @see #isHandled(HttpServletRequest)
      */
     protected boolean handleStatus(int status, boolean error) {
         this.status = status;
-        return isHandled();
+        return isHandled(null);
     }
 
     @Override
@@ -157,11 +156,9 @@ public void setIntHeader(String name, int value) {
     public ServletOutputStream getOutputStream() throws IOException {
         if ( output == null ) output = STREAM;
 
-        if ( isHandled() ) {
-            return super.getOutputStream();
-        }
-        else {
-            // backwards compatibility with isError() then :
+        if ( isHandled(null) ) return super.getOutputStream();
+        else { // TODO get rid of this in 1.2.0
+            // backwards compatibility with isError() :
             return new ServletOutputStream() {
                 @Override
                 public void write(int b) throws IOException {
@@ -175,43 +172,37 @@ public void write(int b) throws IOException {
     public PrintWriter getWriter() throws IOException {
         if ( output == null ) output = WRITER;
 
-        if ( isHandled() ) {
-            // we protect against API limitations as we depend on #getWriter
-            // being functional even if getOutputStream has been called ...
-            if ( output != WRITER ) {
-                String enc = getCharacterEncoding();
-                if ( enc == null ) enc = "UTF-8";
-                return new PrintWriter(new OutputStreamWriter(getOutputStream(), enc));
-            }
-            else {
-                return super.getWriter();
-            }
+        // we protect against API limitations as we depend on #getWriter
+        // being functional even if getOutputStream has been called ...
+        if ( output != WRITER ) {
+            String enc = getCharacterEncoding();
+            if ( enc == null ) enc = "UTF-8";
+            return new PrintWriter(new OutputStreamWriter(getOutputStream(), enc));
         }
         else {
-            // backwards compatibility with isError() then :
-            return new PrintWriter(new OutputStream() {
-                @Override
-                public void write(int b) throws IOException {
-                    // swallow output, because we're going to discard it
-                }
-            });
+            return super.getWriter();
         }
     }
 
     @Override
     public void flushBuffer() throws IOException {
-        if ( isHandled() ) super.flushBuffer();
+        if ( isHandled(null) ) super.flushBuffer();
     }
 
     public boolean isError() {
         return getStatus() >= 400;
     }
 
-    // NOTE: should probably deprecate this one
+    /**
+     * @deprecated no longer to be used from outside
+     * @see #isHandled(HttpServletRequest)
+     */
     public boolean isHandled() {
         return isHandled(null);
     }
 
+    private boolean handled; // once handled - stays handled
+
     /**
      * Response is considered to be handled if a status has been set
      * and it is (by default) not a HTTP NOT FOUND (404) status.
@@ -220,28 +211,36 @@ public boolean isHandled() {
      * @see #handleStatus(int, boolean)
      */
     public boolean isHandled(final HttpServletRequest request) {
+        if ( handled ) return true; // ... once handled always handled !
+
         // setting a header should consider the response to be handled
         if ( ! isStatusSet() ) {
-            if ( ! isHeaderSet() ) return false;
+            if ( ! isHeaderSet() ) {
+                // true by default seems very weird but this is best to cover
+                // "more" containers right out of the box ...
+                // e.g. Jetty https://github.com/jruby/jruby-rack/issues/175
+                return handled = true; // return false;
+            }
 
             // consider HTTP OPTIONS with "Allow" header unhandled :
             if ( request != null && "OPTIONS".equals( request.getMethod() ) ) {
-                final Collection<String> headerNames = getHeaderNamesInternal();
+                final Collection<String> headerNames = getHeaderNamesOrNull();
                 if ( headerNames == null || headerNames.isEmpty() ) {
                     // not to happen but there's all kind of beasts out there
                     return false;
                 }
                 for ( final String headerName : headerNames ) {
                     if ( ! "Allow".equals(headerName) ) {
-                        return true; // not just Allow header - consider handled
+                        return handled = true; // not just Allow header - consider handled
                     }
                 }
                 return false; // OPTIONS with only Allow header set - unhandled
             }
-            return true;
+            return handled = true;
         }
+
         if ( notHandledStatuses.contains( getStatus() ) ) return false;
-        return true;
+        return handled = true;
     }
 
     public Collection<Integer> getNotHandledStatuses() {
@@ -263,8 +262,8 @@ public boolean isOutputAccessed() {
     }
 
     @SuppressWarnings("unchecked")
-    private Collection<String> getHeaderNamesInternal() {
-        // NOTE: getHeaderNames available since 3.0 JRuby-Rack 1.1 still support 2.5
+    private Collection<String> getHeaderNamesOrNull() {
+        // NOTE: getHeaderNames since Servlet API 3.0 JRuby-Rack 1.1 still supports 2.5
         try {
             final Method getHeaderNames = HttpServletResponse.class.getMethod("getHeaderNames");
             return (Collection<String>) getHeaderNames.invoke(this);

src/spec/ruby/rack/servlet/response_capture_spec.rb

@@ -21,8 +21,14 @@
     expect( response_capture.isOutputAccessed ).to be true
   end
 
+  it "is considered handled by default" do
+    # NOTE: weird but this is what some containers need to e.g. serve
+    # static content with RackFilter correctly (e.g. Jetty)
+    expect( response_capture.isHandled ).to be true
+  end
+
   it "is not considered handled by default or when 404 set" do
-    expect( response_capture.isHandled ).to be false
+    #expect( response_capture.isHandled ).to be false
 
     response_capture.setStatus(404)
     expect( response_capture.isHandled ).to be false
@@ -37,10 +43,19 @@
     expect( response_capture.isHandled ).to be true
   end
 
+  it "once considered handled stays handled" do
+    response_capture.setStatus(200)
+    expect( response_capture.isHandled ).to be true
+    # NOTE: quite important since container might have accessed and written to
+    # the real output-stream already ... status change should not happen though
+    response_capture.setStatus(404)
+    expect( response_capture.isHandled ).to be true
+  end
+
   it "is not considered handled when only Allow header is added with OPTIONS" do
     servlet_request.method = 'OPTIONS'
 
-    expect( response_capture.isHandled(servlet_request) ).to be false
+    #expect( response_capture.isHandled(servlet_request) ).to be false
 
     # NOTE: this is what TC's DefaultServlet does on doOptions() :
     response_capture.addHeader "Allow", "GET, POST, OPTIONS"