This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 46ede0a Back-port Range, Content-Range and partial PUT improvements
46ede0a is described below
commit 46ede0a876475e70ed341987e22e864b222b01bd
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jul 2 20:13:05 2019 +0100
Back-port Range, Content-Range and partial PUT improvements
---
.../apache/catalina/servlets/DefaultServlet.java | 195 ++++++++++-----------
.../tomcat/util/http/parser/ContentRange.java | 108 ++++++++++++
.../apache/tomcat/util/http/parser/HttpParser.java | 37 ++++
.../org/apache/tomcat/util/http/parser/Ranges.java | 124 +++++++++++++
.../catalina/servlets/TestDefaultServletPut.java | 185 +++++++++++++++++++
.../servlets/TestDefaultServletRangeRequests.java | 146 +++++++++++++++
.../apache/catalina/startup/SimpleHttpClient.java | 12 +-
webapps/docs/changelog.xml | 21 +++
8 files changed, 723 insertions(+), 105 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 87a9dcb..22f6058 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -80,6 +80,8 @@ import org.apache.catalina.util.URLEncoder;
import org.apache.catalina.webresources.CachedResource;
import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.http.ResponseUtil;
+import org.apache.tomcat.util.http.parser.ContentRange;
+import org.apache.tomcat.util.http.parser.Ranges;
import org.apache.tomcat.util.res.StringManager;
import org.apache.tomcat.util.security.Escape;
import org.apache.tomcat.util.security.PrivilegedGetTccl;
@@ -150,6 +152,8 @@ public class DefaultServlet extends HttpServlet {
*/
protected static final ArrayList<Range> FULL = new ArrayList<>();
+ private static final Range IGNORE = new Range();
+
/**
* MIME multipart separation string
*/
@@ -277,6 +281,12 @@ public class DefaultServlet extends HttpServlet {
*/
protected transient SortManager sortManager;
+ /**
+ * Flag that indicates whether partial PUTs are permitted.
+ */
+ private boolean allowPartialPut = true;
+
+
// --------------------------------------------------------- Public Methods
/**
@@ -377,6 +387,10 @@ public class DefaultServlet extends HttpServlet {
sortManager = new SortManager(sortDirectoriesFirst);
}
}
+
+ if (getServletConfig().getInitParameter("allowPartialPut") != null) {
+ allowPartialPut =
Boolean.parseBoolean(getServletConfig().getInitParameter("allowPartialPut"));
+ }
}
private CompressionFormat[] parseCompressionFormats(String precompressed,
String gzip) {
@@ -608,6 +622,11 @@ public class DefaultServlet extends HttpServlet {
Range range = parseContentRange(req, resp);
+ if (range == null) {
+ // Processing error. parseContentRange() set the error code
+ return;
+ }
+
InputStream resourceInputStream = null;
try {
@@ -615,11 +634,11 @@ public class DefaultServlet extends HttpServlet {
// resource - create a temp. file on the local filesystem to
// perform this operation
// Assume just one range is specified for now
- if (range != null) {
+ if (range == IGNORE) {
+ resourceInputStream = req.getInputStream();
+ } else {
File contentFile = executePartialPut(req, range, path);
resourceInputStream = new FileInputStream(contentFile);
- } else {
- resourceInputStream = req.getInputStream();
}
if (resources.write(path, resourceInputStream, true)) {
@@ -915,7 +934,7 @@ public class DefaultServlet extends HttpServlet {
}
}
- ArrayList<Range> ranges = null;
+ ArrayList<Range> ranges = FULL;
long contentLength = -1L;
if (resource.isDirectory()) {
@@ -941,6 +960,9 @@ public class DefaultServlet extends HttpServlet {
// Parse range specifier
ranges = parseRange(request, response, resource);
+ if (ranges == null) {
+ return;
+ }
// ETag header
response.setHeader("ETag", eTag);
@@ -1019,12 +1041,7 @@ public class DefaultServlet extends HttpServlet {
conversionRequired = false;
}
- if (resource.isDirectory() ||
- isError ||
- ( (ranges == null || ranges.isEmpty())
- && request.getHeader("Range") == null ) ||
- ranges == FULL ) {
-
+ if (resource.isDirectory() || isError || ranges == FULL ) {
// Set the appropriate output headers
if (contentType != null) {
if (debug > 0)
@@ -1381,7 +1398,9 @@ public class DefaultServlet extends HttpServlet {
*
* @param request The servlet request we are processing
* @param response The servlet response we are creating
- * @return the range object
+ * @return the partial content-range, {@code null} if the content-range
+ * header was invalid or {@code #IGNORE} if there is no header to
+ * process
* @throws IOException an IO error occurred
*/
protected Range parseContentRange(HttpServletRequest request,
@@ -1389,44 +1408,37 @@ public class DefaultServlet extends HttpServlet {
throws IOException {
// Retrieving the content-range header (if any is specified
- String rangeHeader = request.getHeader("Content-Range");
+ String contentRangeHeader = request.getHeader("Content-Range");
- if (rangeHeader == null)
- return null;
+ if (contentRangeHeader == null) {
+ return IGNORE;
+ }
- // bytes is the only range unit supported
- if (!rangeHeader.startsWith("bytes")) {
+ if (!allowPartialPut) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
- rangeHeader = rangeHeader.substring(6).trim();
-
- int dashPos = rangeHeader.indexOf('-');
- int slashPos = rangeHeader.indexOf('/');
+ ContentRange contentRange = ContentRange.parse(new
StringReader(contentRangeHeader));
- if (dashPos == -1) {
+ if (contentRange == null) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
- if (slashPos == -1) {
+
+ // bytes is the only range unit supported
+ if (!contentRange.getUnits().equals("bytes")) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
+ // TODO: Remove the internal representation and use Ranges
+ // Convert to internal representation
Range range = new Range();
-
- try {
- range.start = Long.parseLong(rangeHeader.substring(0, dashPos));
- range.end =
- Long.parseLong(rangeHeader.substring(dashPos + 1, slashPos));
- range.length = Long.parseLong
- (rangeHeader.substring(slashPos + 1, rangeHeader.length()));
- } catch (NumberFormatException e) {
- response.sendError(HttpServletResponse.SC_BAD_REQUEST);
- return null;
- }
+ range.start = contentRange.getStart();
+ range.end = contentRange.getEnd();
+ range.length = contentRange.getLength();
if (!range.validate()) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
@@ -1434,7 +1446,6 @@ public class DefaultServlet extends HttpServlet {
}
return range;
-
}
@@ -1444,13 +1455,18 @@ public class DefaultServlet extends HttpServlet {
* @param request The servlet request we are processing
* @param response The servlet response we are creating
* @param resource The resource
- * @return a list of ranges
+ * @return a list of ranges, {@code null} if the range header was invalid
or
+ * {@code #FULL} if the Range header should be ignored.
* @throws IOException an IO error occurred
*/
protected ArrayList<Range> parseRange(HttpServletRequest request,
HttpServletResponse response,
WebResource resource) throws IOException {
+ // Range headers are only valid on GET requests. That implies they are
+ // also valid on HEAD requests. This method is only called by doGet()
+ // and doHead() so no need to check the request method.
+
// Checking If-Range
String headerValue = request.getHeader("If-Range");
@@ -1467,110 +1483,81 @@ public class DefaultServlet extends HttpServlet {
long lastModified = resource.getLastModified();
if (headerValueTime == (-1L)) {
-
// If the ETag the client gave does not match the entity
// etag, then the entire entity is returned.
- if (!eTag.equals(headerValue.trim()))
+ if (!eTag.equals(headerValue.trim())) {
return FULL;
-
+ }
} else {
-
- // If the timestamp of the entity the client got is older than
+ // If the timestamp of the entity the client got differs from
// the last modification date of the entity, the entire entity
// is returned.
- if (lastModified > (headerValueTime + 1000))
+ if (Math.abs(lastModified -headerValueTime) > 1000) {
return FULL;
-
+ }
}
-
}
long fileLength = resource.getContentLength();
if (fileLength == 0) {
- return null;
+ // Range header makes no sense for a zero length resource. Tomcat
+ // therefore opts to ignore it.
+ return FULL;
}
// Retrieving the range header (if any is specified
String rangeHeader = request.getHeader("Range");
if (rangeHeader == null) {
- return null;
+ // No Range header is the same as ignoring any Range header
+ return FULL;
}
- // bytes is the only range unit supported (and I don't see the point
- // of adding new ones).
- if (!rangeHeader.startsWith("bytes")) {
+ Ranges ranges = Ranges.parse(new StringReader(rangeHeader));
+
+ if (ranges == null) {
+ // The Range header is present but not formatted correctly.
+ // Could argue for a 400 response but 416 is more specific.
+ // There is also the option to ignore the (invalid) Range header.
+ // RFC7233#4.4 notes that many servers do ignore the Range header
in
+ // these circumstances but Tomcat has always returned a 416.
response.addHeader("Content-Range", "bytes */" + fileLength);
- response.sendError
- (HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE);
+
response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE);
return null;
}
- rangeHeader = rangeHeader.substring(6);
+ // bytes is the only range unit supported (and I don't see the point
+ // of adding new ones).
+ if (!ranges.getUnits().equals("bytes")) {
+ // RFC7233#3.1 Servers must ignore range units they don't
understand
+ return FULL;
+ }
- // Collection which will contain all the ranges which are successfully
- // parsed.
+ // TODO: Remove the internal representation and use Ranges
+ // Convert to internal representation
ArrayList<Range> result = new ArrayList<>();
- StringTokenizer commaTokenizer = new StringTokenizer(rangeHeader, ",");
-
- // Parsing the range list
- while (commaTokenizer.hasMoreTokens()) {
- String rangeDefinition = commaTokenizer.nextToken().trim();
+ for (Ranges.Entry entry : ranges.getEntries()) {
Range currentRange = new Range();
- currentRange.length = fileLength;
-
- int dashPos = rangeDefinition.indexOf('-');
-
- if (dashPos == -1) {
- response.addHeader("Content-Range", "bytes */" + fileLength);
- response.sendError
- (HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE);
- return null;
- }
-
- if (dashPos == 0) {
-
- try {
- long offset = Long.parseLong(rangeDefinition);
- currentRange.start = fileLength + offset;
- currentRange.end = fileLength - 1;
- } catch (NumberFormatException e) {
- response.addHeader("Content-Range",
- "bytes */" + fileLength);
- response.sendError
- (HttpServletResponse
- .SC_REQUESTED_RANGE_NOT_SATISFIABLE);
- return null;
+ if (entry.getStart() == -1) {
+ currentRange.start = fileLength - entry.getEnd();
+ if (currentRange.start < 0) {
+ currentRange.start = 0;
}
-
+ currentRange.end = fileLength - 1;
+ } else if (entry.getEnd() == -1) {
+ currentRange.start = entry.getStart();
+ currentRange.end = fileLength - 1;
} else {
-
- try {
- currentRange.start = Long.parseLong
- (rangeDefinition.substring(0, dashPos));
- if (dashPos < rangeDefinition.length() - 1)
- currentRange.end = Long.parseLong
- (rangeDefinition.substring
- (dashPos + 1, rangeDefinition.length()));
- else
- currentRange.end = fileLength - 1;
- } catch (NumberFormatException e) {
- response.addHeader("Content-Range",
- "bytes */" + fileLength);
- response.sendError
- (HttpServletResponse
- .SC_REQUESTED_RANGE_NOT_SATISFIABLE);
- return null;
- }
-
+ currentRange.start = entry.getStart();
+ currentRange.end = entry.getEnd();
}
+ currentRange.length = fileLength;
if (!currentRange.validate()) {
response.addHeader("Content-Range", "bytes */" + fileLength);
- response.sendError
- (HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE);
+
response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE);
return null;
}
diff --git a/java/org/apache/tomcat/util/http/parser/ContentRange.java
b/java/org/apache/tomcat/util/http/parser/ContentRange.java
new file mode 100644
index 0000000..59bf071
--- /dev/null
+++ b/java/org/apache/tomcat/util/http/parser/ContentRange.java
@@ -0,0 +1,108 @@
+/*
+ * 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.tomcat.util.http.parser;
+
+import java.io.IOException;
+import java.io.StringReader;
+
+public class ContentRange {
+
+ private final String units;
+ private final long start;
+ private final long end;
+ private final long length;
+
+
+ public ContentRange(String units, long start, long end, long length) {
+ this.units = units;
+ this.start = start;
+ this.end = end;
+ this.length = length;
+ }
+
+
+ public String getUnits() {
+ return units;
+ }
+
+
+ public long getStart() {
+ return start;
+ }
+
+
+ public long getEnd() {
+ return end;
+ }
+
+
+ public long getLength() {
+ return length;
+ }
+
+
+ /**
+ * Parses a Content-Range header from an HTTP header.
+ *
+ * @param input a reader over the header text
+ *
+ * @return the range parsed from the input, or null if not valid
+ *
+ * @throws IOException if there was a problem reading the input
+ */
+ public static ContentRange parse(StringReader input) throws IOException {
+ // Units (required)
+ String units = HttpParser.readToken(input);
+ if (units == null || units.length() == 0) {
+ return null;
+ }
+
+ // Must be followed by '='
+ if (HttpParser.skipConstant(input, "=") == SkipResult.NOT_FOUND) {
+ return null;
+ }
+
+ // Start
+ long start = HttpParser.readLong(input);
+
+ // Must be followed by '-'
+ if (HttpParser.skipConstant(input, "-") == SkipResult.NOT_FOUND) {
+ return null;
+ }
+
+ // End
+ long end = HttpParser.readLong(input);
+
+ // Must be followed by '/'
+ if (HttpParser.skipConstant(input, "/") == SkipResult.NOT_FOUND) {
+ return null;
+ }
+
+ // Length
+ long length = HttpParser.readLong(input);
+
+ // Doesn't matter what we look for, result should be EOF
+ SkipResult skipResult = HttpParser.skipConstant(input, "X");
+
+ if (skipResult != SkipResult.EOF) {
+ // Invalid range
+ return null;
+ }
+
+ return new ContentRange(units, start, end, length);
+ }
+}
diff --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java
b/java/org/apache/tomcat/util/http/parser/HttpParser.java
index 8598321..f76ddcb 100644
--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -405,6 +405,43 @@ public class HttpParser {
}
/**
+ * @return the digits if any were found, the empty string if no data was
+ * found or if data other than digits was found
+ */
+ static String readDigits(Reader input) throws IOException {
+ StringBuilder result = new StringBuilder();
+
+ skipLws(input);
+ input.mark(1);
+ int c = input.read();
+
+ while (c != -1 && isNumeric(c)) {
+ result.append((char) c);
+ input.mark(1);
+ c = input.read();
+ }
+ // Use mark(1)/reset() rather than skip(-1) since skip() is a NOP
+ // once the end of the String has been reached.
+ input.reset();
+
+ return result.toString();
+ }
+
+ /**
+ * @return the number if digits were found, -1 if no data was found
+ * or if data other than digits was found
+ */
+ static long readLong(Reader input) throws IOException {
+ String digits = readDigits(input);
+
+ if (digits.length() == 0) {
+ return -1;
+ }
+
+ return Long.parseLong(digits);
+ }
+
+ /**
* @return the quoted string if one was found, null if data other than a
* quoted string was found or null if the end of data was reached
* before the quoted string was terminated
diff --git a/java/org/apache/tomcat/util/http/parser/Ranges.java
b/java/org/apache/tomcat/util/http/parser/Ranges.java
new file mode 100644
index 0000000..c937eb5
--- /dev/null
+++ b/java/org/apache/tomcat/util/http/parser/Ranges.java
@@ -0,0 +1,124 @@
+/*
+ * 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.tomcat.util.http.parser;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class Ranges {
+
+ private final String units;
+ private final List<Entry> entries;
+
+
+ private Ranges(String units, List<Entry> entries) {
+ this.units = units;
+ this.entries = Collections.unmodifiableList(entries);
+ }
+
+
+ public List<Entry> getEntries() {
+ return entries;
+ }
+
+ public String getUnits() {
+ return units;
+ }
+
+
+ public static class Entry {
+
+ private final long start;
+ private final long end;
+
+
+ public Entry(long start, long end) {
+ this.start = start;
+ this.end = end;
+ }
+
+
+ public long getStart() {
+ return start;
+ }
+
+
+ public long getEnd() {
+ return end;
+ }
+ }
+
+
+ /**
+ * Parses a Range header from an HTTP header.
+ *
+ * @param input a reader over the header text
+ *
+ * @return a set of ranges parsed from the input, or null if not valid
+ *
+ * @throws IOException if there was a problem reading the input
+ */
+ public static Ranges parse(StringReader input) throws IOException {
+
+ // Units (required)
+ String units = HttpParser.readToken(input);
+ if (units == null || units.length() == 0) {
+ return null;
+ }
+
+ // Must be followed by '='
+ if (HttpParser.skipConstant(input, "=") == SkipResult.NOT_FOUND) {
+ return null;
+ }
+
+ // Range entries
+ List<Entry> entries = new ArrayList<>();
+
+ SkipResult skipResult;
+ do {
+ long start = HttpParser.readLong(input);
+ // Must be followed by '-'
+ if (HttpParser.skipConstant(input, "-") == SkipResult.NOT_FOUND) {
+ return null;
+ }
+ long end = HttpParser.readLong(input);
+
+ if (start == -1 && end == -1) {
+ // Invalid range
+ return null;
+ }
+
+ entries.add(new Entry(start, end));
+
+ skipResult = HttpParser.skipConstant(input, ",");
+ if (skipResult == SkipResult.NOT_FOUND) {
+ // Invalid range
+ return null;
+ }
+ } while (skipResult == SkipResult.FOUND);
+
+ // There must be at least one entry
+ if (entries.size() == 0) {
+ return null;
+ }
+
+ return new Ranges(units, entries);
+ }
+}
diff --git a/test/org/apache/catalina/servlets/TestDefaultServletPut.java
b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
new file mode 100644
index 0000000..915c448
--- /dev/null
+++ b/test/org/apache/catalina/servlets/TestDefaultServletPut.java
@@ -0,0 +1,185 @@
+/*
+ * 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.catalina.servlets;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import static org.apache.catalina.startup.SimpleHttpClient.CRLF;
+import org.apache.catalina.Context;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.ExpandWar;
+import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+@RunWith(Parameterized.class)
+public class TestDefaultServletPut extends TomcatBaseTest {
+
+ private static final String START_TEXT= "Starting text";
+ private static final String START_LEN =
Integer.toString(START_TEXT.length());
+ private static final String PATCH_TEXT= "Ending *";
+ private static final String PATCH_LEN =
Integer.toString(PATCH_TEXT.length());
+ private static final String END_TEXT= "Ending * text";
+
+ @Parameterized.Parameters(name = "{index} rangeHeader [{0}]")
+ public static Collection<Object[]> parameters() {
+ List<Object[]> parameterSets = new ArrayList<>();
+
+ // Valid partial PUT
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes=0-" + PATCH_LEN + "/" + START_LEN +
CRLF, Boolean.TRUE, END_TEXT });
+ // Full PUT
+ parameterSets.add(new Object[] {
+ "", null, PATCH_TEXT });
+ // Invalid range
+ parameterSets.add(new Object[] {
+ "Content-Range: apples=0-" + PATCH_LEN + "/" + START_LEN +
CRLF, Boolean.FALSE, START_TEXT });
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes00-" + PATCH_LEN + "/" + START_LEN +
CRLF, Boolean.FALSE, START_TEXT });
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes=9-7/" + START_LEN + CRLF, Boolean.FALSE,
START_TEXT });
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes=-7/" + START_LEN + CRLF, Boolean.FALSE,
START_TEXT });
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes=9-/" + START_LEN + CRLF, Boolean.FALSE,
START_TEXT });
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes=9-X/" + START_LEN + CRLF, Boolean.FALSE,
START_TEXT });
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes=0-5/" + CRLF, Boolean.FALSE, START_TEXT
});
+ parameterSets.add(new Object[] {
+ "Content-Range: bytes=0-5/0x5" + CRLF, Boolean.FALSE,
START_TEXT });
+
+ return parameterSets;
+ }
+
+
+ private File tempDocBase;
+
+ @Parameter(0)
+ public String contentRangeHeader;
+
+ @Parameter(1)
+ public Boolean contentRangeHeaderValid;
+
+ @Parameter(2)
+ public String expectedEndText;
+
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ tempDocBase =
Files.createTempDirectory(getTemporaryDirectory().toPath(), "put").toFile();
+ }
+
+
+ /*
+ * Replaces the text at the start of START_TEXT with PATCH_TEXT.
+ */
+ @Test
+ public void testPut() throws Exception {
+ // Configure a web app with a read/write default servlet
+ Tomcat tomcat = getTomcatInstance();
+ Context ctxt = tomcat.addContext("", tempDocBase.getAbsolutePath());
+
+ Wrapper w = Tomcat.addServlet(ctxt, "default",
DefaultServlet.class.getName());
+ w.addInitParameter("readonly", "false");
+ ctxt.addServletMappingDecoded("/", "default");
+
+ tomcat.start();
+
+ // Disable caching
+ ctxt.getResources().setCachingAllowed(false);
+
+ // Full PUT
+ PutClient putClient = new PutClient(getPort());
+
+ putClient.setRequest(new String[] {
+ "PUT /test.txt HTTP/1.1" + CRLF +
+ "Host: localhost:" + getPort() + CRLF +
+ "Content-Length: " + START_LEN + CRLF +
+ CRLF +
+ START_TEXT
+ });
+ putClient.connect();
+ putClient.processRequest(false);
+ Assert.assertTrue(putClient.isResponse201());
+ putClient.disconnect();
+
+ putClient.reset();
+
+ // Partial PUT
+ putClient.connect();
+ putClient.setRequest(new String[] {
+ "PUT /test.txt HTTP/1.1" + CRLF +
+ "Host: localhost:" + getPort() + CRLF +
+ contentRangeHeader +
+ "Content-Length: " + PATCH_LEN + CRLF +
+ CRLF +
+ PATCH_TEXT
+ });
+ putClient.processRequest(false);
+ if (contentRangeHeaderValid == null) {
+ // Not present (so will do a full PUT, replacing the existing)
+ Assert.assertTrue(putClient.isResponse204());
+ } else if (contentRangeHeaderValid.booleanValue()) {
+ // Valid
+ Assert.assertTrue(putClient.isResponse204());
+ } else {
+ // Not valid
+ Assert.assertTrue(putClient.isResponse400());
+ }
+
+ // Check for the final resource
+ String path = "http://localhost:"; + getPort() + "/test.txt";
+ ByteChunk responseBody = new ByteChunk();
+
+ int rc = getUrl(path, responseBody, null);
+
+ Assert.assertEquals(200, rc);
+ Assert.assertEquals(expectedEndText, responseBody.toString());
+ }
+
+
+ @Override
+ public void tearDown() {
+ ExpandWar.deleteDir(tempDocBase, false);
+ }
+
+
+ private static class PutClient extends SimpleHttpClient {
+
+ public PutClient(int port) {
+ setPort(port);
+ }
+
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return false;
+ }
+ }
+}
diff --git
a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
new file mode 100644
index 0000000..c50cc4b
--- /dev/null
+++ b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
@@ -0,0 +1,146 @@
+/*
+ * 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.catalina.servlets;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+@RunWith(Parameterized.class)
+public class TestDefaultServletRangeRequests extends TomcatBaseTest {
+
+ @Parameterized.Parameters(name = "{index} rangeHeader [{0}]")
+ public static Collection<Object[]> parameters() {
+
+ // Get the length of the file used for this test
+ // It varies by platform due to line-endings
+ File index = new File("test/webapp/index.html");
+ long len = index.length();
+ String strLen = Long.toString(len);
+
+ List<Object[]> parameterSets = new ArrayList<>();
+
+ parameterSets.add(new Object[] { "", Integer.valueOf(200), strLen, ""
});
+ // Invalid
+ parameterSets.add(new Object[] { "bytes", Integer.valueOf(416), "",
"*/" + len });
+ parameterSets.add(new Object[] { "bytes=", Integer.valueOf(416), "",
"*/" + len });
+ // Invalid with unknown type
+ parameterSets.add(new Object[] { "unknown", Integer.valueOf(416), "",
"*/" + len });
+ parameterSets.add(new Object[] { "unknown=", Integer.valueOf(416), "",
"*/" + len });
+ // Invalid ranges
+ parameterSets.add(new Object[] { "bytes=-", Integer.valueOf(416), "",
"*/" + len });
+ parameterSets.add(new Object[] { "bytes=10-b", Integer.valueOf(416),
"", "*/" + len });
+ parameterSets.add(new Object[] { "bytes=b-10", Integer.valueOf(416),
"", "*/" + len });
+ // Invalid no equals
+ parameterSets.add(new Object[] { "bytes 1-10", Integer.valueOf(416),
"", "*/" + len });
+ parameterSets.add(new Object[] { "bytes1-10", Integer.valueOf(416),
"", "*/" + len });
+ parameterSets.add(new Object[] { "bytes10-", Integer.valueOf(416), "",
"*/" + len });
+ parameterSets.add(new Object[] { "bytes-10", Integer.valueOf(416), "",
"*/" + len });
+ // Unknown types
+ parameterSets.add(new Object[] { "unknown=1-2", Integer.valueOf(200),
strLen, "" });
+ parameterSets.add(new Object[] { "bytesX=1-2", Integer.valueOf(200),
strLen, "" });
+ parameterSets.add(new Object[] { "Xbytes=1-2", Integer.valueOf(200),
strLen, "" });
+ // Valid range
+ parameterSets.add(new Object[] { "bytes=0-9", Integer.valueOf(206),
"10", "0-9/" + len });
+ parameterSets.add(new Object[] { "bytes=-100", Integer.valueOf(206),
"100", (len - 100) + "-" + (len - 1) + "/" + len });
+ parameterSets.add(new Object[] { "bytes=100-", Integer.valueOf(206),
"" + (len - 100), "100-" + (len - 1) + "/" + len });
+ // Valid range (too much)
+ parameterSets.add(new Object[] { "bytes=0-1000", Integer.valueOf(206),
strLen, "0-" + (len - 1) + "/" + len });
+ parameterSets.add(new Object[] { "bytes=-1000", Integer.valueOf(206),
strLen, "0-" + (len - 1) + "/" + len });
+ return parameterSets;
+ }
+
+ @Parameter(0)
+ public String rangeHeader;
+ @Parameter(1)
+ public int responseCodeExpected;
+ @Parameter(2)
+ public String contentLengthExpected;
+ @Parameter(3)
+ public String responseRangeExpected;
+
+ @Test
+ public void testRange() throws Exception {
+
+ Tomcat tomcat = getTomcatInstance();
+
+ File appDir = new File("test/webapp");
+ Context ctxt = tomcat.addContext("", appDir.getAbsolutePath());
+
+ Tomcat.addServlet(ctxt, "default", DefaultServlet.class.getName());
+ ctxt.addServletMappingDecoded("/", "default");
+
+ tomcat.start();
+
+ // Set up parameters
+ String path = "http://localhost:"; + getPort() + "/index.html";
+ ByteChunk responseBody = new ByteChunk();
+ Map<String,List<String>> responseHeaders = new HashMap<>();
+
+ int rc = getUrl(path, responseBody, buildRangeHeader(rangeHeader),
responseHeaders);
+
+ // Check the result
+ Assert.assertEquals(responseCodeExpected, rc);
+
+ if (contentLengthExpected.length() > 0) {
+ String contentLength =
responseHeaders.get("Content-Length").get(0);
+ Assert.assertEquals(contentLengthExpected, contentLength);
+ }
+
+ if (responseRangeExpected.length() > 0) {
+ String responseRange = null;
+ List<String> headerValues = responseHeaders.get("Content-Range");
+ if (headerValues != null && headerValues.size() == 1) {
+ responseRange = headerValues.get(0);
+ }
+ Assert.assertEquals("bytes " + responseRangeExpected,
responseRange);
+ }
+ }
+
+
+ private static Map<String,List<String>> buildRangeHeader(String...
headerValues) {
+ Map<String,List<String>> requestHeaders = new HashMap<>();
+ List<String> values = new ArrayList<>();
+ for (String headerValue : headerValues) {
+ if (headerValue.length() > 0) {
+ values.add(headerValue);
+ }
+ }
+
+ if (values.size() == 0) {
+ return null;
+ }
+
+ requestHeaders.put("range", values);
+
+ return requestHeaders;
+ }
+}
diff --git a/test/org/apache/catalina/startup/SimpleHttpClient.java
b/test/org/apache/catalina/startup/SimpleHttpClient.java
index 05cbb83..ec0af7d 100644
--- a/test/org/apache/catalina/startup/SimpleHttpClient.java
+++ b/test/org/apache/catalina/startup/SimpleHttpClient.java
@@ -48,6 +48,8 @@ public abstract class SimpleHttpClient {
public static final String INFO_100 = "HTTP/1.1 100 ";
public static final String OK_200 = "HTTP/1.1 200 ";
+ public static final String CREATED_201 = "HTTP/1.1 201 ";
+ public static final String NOCONTENT_204 = "HTTP/1.1 204 ";
public static final String REDIRECT_302 = "HTTP/1.1 302 ";
public static final String REDIRECT_303 = "HTTP/1.1 303 ";
public static final String FAIL_400 = "HTTP/1.1 400 ";
@@ -411,6 +413,14 @@ public abstract class SimpleHttpClient {
return responseLineStartsWith(OK_200);
}
+ public boolean isResponse201() {
+ return responseLineStartsWith(CREATED_201);
+ }
+
+ public boolean isResponse204() {
+ return responseLineStartsWith(NOCONTENT_204);
+ }
+
public boolean isResponse302() {
return responseLineStartsWith(REDIRECT_302);
}
@@ -460,4 +470,4 @@ public abstract class SimpleHttpClient {
}
public abstract boolean isResponseBodyOK();
-}
\ No newline at end of file
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index cc15118..bccc8b4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -80,6 +80,27 @@
that changes to <code>$CATALINA_BASE/conf/tomcat-users.xml</code> will
now take effect a short time after the file is saved. (markt)
</add>
+ <fix>
+ Improve parsing of Range request headers. (markt)
+ </fix>
+ <fix>
+ Range headers that specify a range unit Tomcat does not recognise
should
+ be ignored rather than triggering a 416 response. Based on a pull
+ request by zhanhb. (markt)
+ </fix>
+ <fix>
+ When comparing a date from a <code>If-Range</code> header, an exact
+ match is required. Based on a pull request by zhanhb. (markt)
+ </fix>
+ <fix>
+ Add an option to the default servlet to disable processing of PUT
+ requests with Content-Range headers as partial PUTs. The default
+ behaviour (processing as partial PUT) is unchanged. Based on a pull
+ request by zhanhb. (markt)
+ </fix>
+ <fix>
+ Improve parsing of Content-Range headers. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
