You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2013/11/22 09:25:29 UTC
svn commit: r1544432 - in /sling/trunk/bundles/servlets/get/src:
main/java/org/apache/sling/servlets/get/impl/helpers/
test/java/org/apache/sling/servlets/get/impl/helpers/
Author: bdelacretaz
Date: Fri Nov 22 08:25:29 2013
New Revision: 1544432
URL: http://svn.apache.org/r1544432
Log:
SLING-3255 - StreamRendererServlet made incorrect assumptions about BufferedInputStream - contributed by Jukka Zitting, thanks!
Added:
sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/
sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServletTest.java (with props)
Modified:
sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServlet.java
Modified: sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServlet.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServlet.java?rev=1544432&r1=1544431&r2=1544432&view=diff
==============================================================================
--- sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServlet.java (original)
+++ sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServlet.java Fri Nov 22 08:25:29 2013
@@ -502,15 +502,13 @@ public class StreamRendererServlet exten
* Copy the contents of the specified input stream to the specified
* output stream.
*
- * @param cacheEntry The cache entry for the source resource
+ * @param istream The input stream to read from
* @param ostream The output stream to write to
* @param range Range the client wanted to retrieve
* @exception IOException if an input/output error occurs
*/
- private void copy(InputStream resourceInputStream, OutputStream ostream,
+ private void copy(InputStream istream, OutputStream ostream,
Range range) throws IOException {
-
- InputStream istream = new BufferedInputStream(resourceInputStream, IO_BUFFER_SIZE);
IOException exception = copyRange(istream, ostream, range.start, range.end);
// Rethrow any exception that has occurred
@@ -531,44 +529,48 @@ public class StreamRendererServlet exten
*/
private IOException copyRange(InputStream istream,
OutputStream ostream, long start, long end) {
-
log.debug("copyRange: Serving bytes: {}-{}", start, end);
+ return staticCopyRange(istream, ostream, start, end);
+ }
+ // static, package-private method to make unit testing easier
+ static IOException staticCopyRange(InputStream istream,
+ OutputStream ostream, long start, long end) {
try {
- long skipped = istream.skip(start);
- if (skipped < start) {
- return new IOException("Failed to skip " + start
- + " bytes; only skipped " + skipped + " bytes");
- }
- } catch (IOException e) {
- return e;
- }
+ long position = 0;
+ byte buffer[] = new byte[IO_BUFFER_SIZE];
- IOException exception = null;
- long bytesToRead = end - start + 1;
+ while (position < start) {
+ long skipped = istream.skip(start - position);
+ if (skipped == 0) {
+ // skip() may return zero if for whatever reason it wasn't
+ // able to advance the stream. In such cases we need to
+ // fall back to read() to force the skipping of bytes.
+ int len = (int) Math.min(start - position, buffer.length);
+ skipped = istream.read(buffer, 0, len);
+ if (skipped == -1) {
+ return new IOException("Failed to skip " + start
+ + " bytes; only skipped " + position + " bytes");
+ }
+ }
+ position += skipped;
+ }
- byte buffer[] = new byte[IO_BUFFER_SIZE];
- int len = buffer.length;
- while ((bytesToRead > 0) && (len >= buffer.length)) {
- try {
- len = istream.read(buffer);
- if (bytesToRead >= len) {
+ while (position < end) {
+ int len = (int) Math.min(end - position, buffer.length);
+ int read = istream.read(buffer, 0, len);
+ if (read != -1) {
+ position += read;
ostream.write(buffer, 0, len);
- bytesToRead -= len;
} else {
- ostream.write(buffer, 0, (int) bytesToRead);
- bytesToRead = 0;
+ break;
}
- } catch (IOException e) {
- exception = e;
- len = -1;
}
- if (len < buffer.length) {
- break;
- }
- }
- return exception;
+ return null;
+ } catch (IOException e) {
+ return e;
+ }
}
/**
Added: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServletTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServletTest.java?rev=1544432&view=auto
==============================================================================
--- sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServletTest.java (added)
+++ sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServletTest.java Fri Nov 22 08:25:29 2013
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.servlets.get.impl.helpers;
+
+import java.io.BufferedInputStream;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.util.Random;
+
+import org.apache.sling.servlets.get.impl.helpers.StreamRendererServlet;
+
+import junit.framework.TestCase;
+
+public class StreamRendererServletTest extends TestCase {
+
+ public void testCopyRange() {
+ assertCopyRange(1234);
+ assertCopyRange(4321);
+ }
+
+ private void assertCopyRange(long seed) {
+ Random random = new Random(seed);
+
+ // generate some random test data
+ byte[] expected = new byte[random.nextInt(10000)]; // >> IO_BUFFER_SIZE
+ random.nextBytes(expected);
+
+ // Check some simple cases ...
+ assertCopyRange(expected, 0, 0);
+ assertCopyRange(expected, 0, 1);
+ assertCopyRange(expected, 0, expected.length);
+
+ // ... and a few randomly selected ones
+ int n = random.nextInt(100);
+ for (int i = 0; i < n; i++) {
+ int a = random.nextInt(expected.length);
+ int b = random.nextInt(expected.length);
+ if (a > b) {
+ int x = a;
+ a = b;
+ b = x;
+ }
+ assertCopyRange(expected, a, b);
+ }
+ }
+
+ private void assertCopyRange(byte[] expected, int a, int b) {
+ assertCopyRange(expected, new ByteArrayInputStream(expected), a, b);
+ // with BufferedInputStream
+ assertCopyRange(expected, new BufferedInputStream(new ByteArrayInputStream(expected)), a, b);
+ // without available()
+ assertCopyRange(expected, new ByteArrayInputStream(expected) {
+ @Override
+ public synchronized int available() {
+ return 0;
+ }
+ }, a, b);
+ // with BufferedInputStream and without available()
+ assertCopyRange(expected, new BufferedInputStream(new ByteArrayInputStream(expected) {
+ @Override
+ public synchronized int available() {
+ return 0;
+ }
+ }), a, b);
+ }
+
+ private void assertCopyRange(
+ byte[] expected, InputStream input, int a, int b) {
+ ByteArrayOutputStream output = new ByteArrayOutputStream();
+ StreamRendererServlet.staticCopyRange(
+ new ByteArrayInputStream(expected), output, a, b);
+
+ byte[] actual = output.toByteArray();
+ assertEquals(b - a, actual.length);
+ for (int i = a; i < b; i++) {
+ assertEquals(expected[i], actual[i - a]);
+ }
+ }
+}
Propchange: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServletTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/helpers/StreamRendererServletTest.java
------------------------------------------------------------------------------
svn:keywords = Author Date Id Revision Rev URL