HTTP range requests cannot be reliably served when a Writer is in use so prevent...

This is kkolinko's patch that stemmed from a private discussion about a TCK test.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1086783 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java
index 5cd3510..6086fb1 100644 (file)
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -876,6 +876,8 @@ public class DefaultServlet
                         || (contentType.startsWith("text"))
                         || (contentType.endsWith("xml")) ) {
                     writer = response.getWriter();
+                    // Cannot reliably serve partial content with a Writer
+                    ranges = FULL;
                 } else {
                     throw e;
                 }
@@ -896,7 +898,8 @@ public class DefaultServlet
                         contentType + "'");
                 response.setContentType(contentType);
             }
-            if ((cacheEntry.resource != null) && (contentLength >= 0)) {
+            if ((cacheEntry.resource != null) && (contentLength >= 0)
+                    && (!serveContent || ostream != null)) {
                 if (debug > 0)
                     log("DefaultServlet.serveFile:  contentLength=" +
                         contentLength);
@@ -974,7 +977,8 @@ public class DefaultServlet
                         if (!checkSendfile(request, response, cacheEntry, range.end - range.start + 1, range))
                             copy(cacheEntry, ostream, range);
                     } else {
-                        copy(cacheEntry, writer, range);
+                        // we should not get here
+                        throw new IllegalStateException();
                     }
                 }
 
@@ -993,8 +997,8 @@ public class DefaultServlet
                         copy(cacheEntry, ostream, ranges.iterator(),
                              contentType);
                     } else {
-                        copy(cacheEntry, writer, ranges.iterator(),
-                             contentType);
+                        // we should not get here
+                        throw new IllegalStateException();
                     }
                 }
 
@@ -1939,44 +1943,6 @@ public class DefaultServlet
      * (even in the face of an exception).
      *
      * @param cacheEntry The cache entry for the source resource
-     * @param writer The writer to write to
-     * @param range Range the client wanted to retrieve
-     * @exception IOException if an input/output error occurs
-     */
-    protected void copy(CacheEntry cacheEntry, PrintWriter writer,
-                      Range range)
-        throws IOException {
-
-        IOException exception = null;
-
-        InputStream resourceInputStream = cacheEntry.resource.streamContent();
-
-        Reader reader;
-        if (fileEncoding == null) {
-            reader = new InputStreamReader(resourceInputStream);
-        } else {
-            reader = new InputStreamReader(resourceInputStream,
-                                           fileEncoding);
-        }
-
-        exception = copyRange(reader, writer, range.start, range.end);
-
-        // Clean up the input stream
-        reader.close();
-
-        // Rethrow any exception that has occurred
-        if (exception != null)
-            throw exception;
-
-    }
-
-
-    /**
-     * Copy the contents of the specified input stream to the specified
-     * output stream, and ensure that both streams are closed before returning
-     * (even in the face of an exception).
-     *
-     * @param cacheEntry The cache entry for the source resource
      * @param ostream The output stream to write to
      * @param ranges Enumeration of the ranges the client wanted to retrieve
      * @param contentType Content type of the resource
@@ -2029,65 +1995,6 @@ public class DefaultServlet
      * output stream, and ensure that both streams are closed before returning
      * (even in the face of an exception).
      *
-     * @param cacheEntry The cache entry for the source resource
-     * @param writer The writer to write to
-     * @param ranges Enumeration of the ranges the client wanted to retrieve
-     * @param contentType Content type of the resource
-     * @exception IOException if an input/output error occurs
-     */
-    protected void copy(CacheEntry cacheEntry, PrintWriter writer,
-                      Iterator<Range> ranges, String contentType)
-        throws IOException {
-
-        IOException exception = null;
-
-        while ( (exception == null) && (ranges.hasNext()) ) {
-
-            InputStream resourceInputStream = cacheEntry.resource.streamContent();
-            
-            Reader reader;
-            if (fileEncoding == null) {
-                reader = new InputStreamReader(resourceInputStream);
-            } else {
-                reader = new InputStreamReader(resourceInputStream,
-                                               fileEncoding);
-            }
-
-            Range currentRange = ranges.next();
-
-            // Writing MIME header.
-            writer.println();
-            writer.println("--" + mimeSeparation);
-            if (contentType != null)
-                writer.println("Content-Type: " + contentType);
-            writer.println("Content-Range: bytes " + currentRange.start
-                           + "-" + currentRange.end + "/"
-                           + currentRange.length);
-            writer.println();
-
-            // Printing content
-            exception = copyRange(reader, writer, currentRange.start,
-                                  currentRange.end);
-
-            reader.close();
-
-        }
-
-        writer.println();
-        writer.print("--" + mimeSeparation + "--");
-
-        // Rethrow any exception that has occurred
-        if (exception != null)
-            throw exception;
-
-    }
-
-
-    /**
-     * Copy the contents of the specified input stream to the specified
-     * output stream, and ensure that both streams are closed before returning
-     * (even in the face of an exception).
-     *
      * @param istream The input stream to read from
      * @param ostream The output stream to write to
      * @return Exception which occurred during processing
@@ -2205,60 +2112,6 @@ public class DefaultServlet
     }
 
 
-    /**
-     * Copy the contents of the specified input stream to the specified
-     * output stream, and ensure that both streams are closed before returning
-     * (even in the face of an exception).
-     *
-     * @param reader The reader to read from
-     * @param writer The writer to write to
-     * @param start Start of the range which will be copied
-     * @param end End of the range which will be copied
-     * @return Exception which occurred during processing
-     */
-    protected IOException copyRange(Reader reader, PrintWriter writer,
-                                  long start, long end) {
-
-        long skipped = 0;
-        try {
-            skipped = reader.skip(start);
-        } catch (IOException e) {
-            return e;
-        }
-        if (skipped < start) {
-            return new IOException(sm.getString("defaultservlet.skipfail",
-                    Long.valueOf(skipped), Long.valueOf(start)));
-        }
-
-        IOException exception = null;
-        long bytesToRead = end - start + 1;
-
-        char buffer[] = new char[input];
-        int len = buffer.length;
-        while ( (bytesToRead > 0) && (len >= buffer.length)) {
-            try {
-                len = reader.read(buffer);
-                if (bytesToRead >= len) {
-                    writer.write(buffer, 0, len);
-                    bytesToRead -= len;
-                } else {
-                    writer.write(buffer, 0, (int) bytesToRead);
-                    bytesToRead = 0;
-                }
-            } catch (IOException e) {
-                exception = e;
-                len = -1;
-            }
-            if (len < buffer.length)
-                break;
-        }
-
-        return exception;
-
-    }
-
-
-
     // ------------------------------------------------------ Range Inner Class
 
 

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3a80e32..09e61a3 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -99,6 +99,10 @@
         as integrated Windows authentication. This is a work in progress. See
         the documentation for details. (markt)
       </add>
+      <fix>
+        HTTP range requests cannot be reliably served when a Writer is in use so
+        prevent the DefaultServlet from attempting to do so. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">