This is an automated email from the ASF dual-hosted git repository. reiern70 pushed a commit to branch byte-ranges-1676 in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 4f8bd1beaaa0cefe92ca45b298fae2bdab0248ac Author: reiern70 <[email protected]> AuthorDate: Tue Oct 20 13:51:32 2026 -0500 Handle suffix, invalid and unsatisfiable byte ranges AbstractResource understood only the <first>-<last> form of a Range header, so the other forms RFC 9110 defines were answered wrongly: - a suffix range (bytes=-5) was served from the start of the content instead of being its last 5 bytes (section 14.1.2); - a range whose last-pos is below its first-pos (bytes=20-10) was served from first-pos to the end, although it is invalid (section 14.1.1); - a range starting at or past the end of the content (bytes=30- on 25 bytes) was answered with 206 and an empty body instead of 416 Range Not Satisfiable with Content-Range: bytes */25 (section 15.5.17); - a last-pos beyond the content was copied into the Content-Range header (bytes 0-99/25), while section 14.4 requires it to be below the complete length (bytes 0-24/25). The header is parsed before the content length is known, so a suffix range is now kept in CONTENT_RANGE_STARTBYTE as a negative start byte and resolved against the length both in setResponseContentRangeHeaderFields(), which writes the headers, and in PartWriterCallback, which writes the body. bytes=-0 selects nothing and is kept as a start past any end, so it is unsatisfiable like bytes=30-. An invalid range is ignored and the whole content is served with 200, as section 14.2 allows; its meta data is cleared so it cannot inherit a range parsed earlier in the same request cycle. bytes=-, which names neither a start nor a suffix length, is invalid too and now gets 200 where it got 206 for the whole content. A range on empty content is ignored as well, since no Content-Range can describe a part of it. An unsatisfiable range is answered with 416 and Content-Range: bytes */<length> by setResponseContentRangeHeaderFields(). The write callback still runs, so PartWriterCallback closes the stream it was given; it writes nothing for a start at or past the end, even when the stream skips fewer bytes than asked. Applications that read CONTENT_RANGE_STARTBYTE themselves now see a negative value for a suffix range where they saw 0 before, a value past the end for bytes=-0, and no meta data for an invalid range. Passing the values on to PartWriterCallback, as PackageResource and FileSystemResource do, keeps working unchanged; code that skips or writes a Content-Range from the raw values has to resolve a negative start against the content length first. GitHub issue #1676 --- .../resource/PackageResourceReferenceTest.java | 39 ++++++++ .../request/resource/PartWriterCallbackTest.java | 110 +++++++++++++++++++++ .../wicket/request/resource/AbstractResource.java | 83 ++++++++++++---- .../request/resource/PartWriterCallback.java | 27 +++-- 4 files changed, 229 insertions(+), 30 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java index 3e23066025..762a672dd6 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java @@ -43,6 +43,8 @@ import org.apache.wicket.markup.IMarkupResourceStreamProvider; import org.apache.wicket.markup.html.WebPage; import org.apache.wicket.protocol.http.mock.MockHttpServletRequest; import org.apache.wicket.protocol.http.mock.MockHttpServletResponse; +import org.apache.wicket.protocol.http.servlet.ServletWebRequest; +import org.apache.wicket.protocol.http.servlet.ServletWebResponse; import org.apache.wicket.request.Request; import org.apache.wicket.request.Response; import org.apache.wicket.request.resource.AbstractResource.ContentRangeType; @@ -298,6 +300,43 @@ class PackageResourceReferenceTest extends WicketTestCase assertEquals("var_style_en.txt", makeRangeRequest("bytes=9-")); assertEquals("resource_var_style_en.txt", makeRangeRequest("bytes=-")); assertEquals("resource_var_style_en.txt", makeRangeRequest("bytes=-25")); + assertEquals("n.txt", makeRangeRequest("bytes=-5")); + assertEquals("resource_var_style_en.txt", makeRangeRequest("bytes=-30")); + assertEquals("resource_var_style_en.txt", makeRangeRequest("bytes=0-99")); + assertEquals("resource_var_style_en.txt", makeRangeRequest("bytes=20-10")); + } + + /** + * Status and Content-Range of the byte range forms in RFC 9110, section 14.1.2 + */ + @Test + void testContentRangeResponse() + { + assertRangeResponse("bytes=3-7", 206, "bytes 3-7/25", "ource"); + assertRangeResponse("bytes=-5", 206, "bytes 20-24/25", "n.txt"); + assertRangeResponse("bytes=-30", 206, "bytes 0-24/25", "resource_var_style_en.txt"); + assertRangeResponse("bytes=0-99", 206, "bytes 0-24/25", "resource_var_style_en.txt"); + assertRangeResponse("bytes=20-10", 200, null, "resource_var_style_en.txt"); + assertRangeResponse("bytes=25-", 416, "bytes */25", ""); + assertRangeResponse("bytes=30-", 416, "bytes */25", ""); + assertRangeResponse("bytes=-0", 416, "bytes */25", ""); + } + + private void assertRangeResponse(String range, int status, String contentRange, String body) + { + ResourceReference reference = new PackageResourceReference(scope, "resource.txt", + locales[1], styles[1], variations[1]); + + ServletWebRequest request = (ServletWebRequest)tester.getRequestCycle().getRequest(); + MockHttpServletRequest httpRequest = (MockHttpServletRequest)request.getContainerRequest(); + httpRequest.setHeader("range", range); + MockHttpServletResponse httpResponse = new MockHttpServletResponse(httpRequest); + reference.getResource() + .respond(new Attributes(request, new ServletWebResponse(request, httpResponse))); + + assertEquals(status, httpResponse.getStatus(), range); + assertEquals(contentRange, httpResponse.getHeader("Content-Range"), range); + assertEquals(body, new String(httpResponse.getBinaryContent()), range); } private String makeRangeRequest(String range) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/request/resource/PartWriterCallbackTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/request/resource/PartWriterCallbackTest.java new file mode 100644 index 0000000000..7d774c34da --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/request/resource/PartWriterCallbackTest.java @@ -0,0 +1,110 @@ +/* + * 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.wicket.request.resource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import org.apache.wicket.protocol.http.mock.MockHttpServletRequest; +import org.apache.wicket.protocol.http.mock.MockHttpServletResponse; +import org.apache.wicket.protocol.http.servlet.ServletWebRequest; +import org.apache.wicket.protocol.http.servlet.ServletWebResponse; +import org.apache.wicket.request.cycle.RequestCycle; +import org.apache.wicket.request.resource.AbstractResource.ContentRangeType; +import org.apache.wicket.request.resource.IResource.Attributes; +import org.apache.wicket.response.ByteArrayResponse; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.jupiter.api.Test; + +/** + * Tests for {@link PartWriterCallback} + */ +class PartWriterCallbackTest extends WicketTestCase +{ + private static final byte[] CONTENT = "0123456789".getBytes(StandardCharsets.US_ASCII); + + @Test + void unsatisfiableRangeClosesTheStream() + { + TrackingInputStream stream = new TrackingInputStream(Integer.MAX_VALUE); + AbstractResource resource = new AbstractResource() + { + @Override + protected ResourceResponse newResourceResponse(Attributes attributes) + { + RequestCycle cycle = RequestCycle.get(); + ResourceResponse response = new ResourceResponse(); + response.setContentLength(CONTENT.length); + response.setAcceptRange(ContentRangeType.BYTES); + response.setWriteCallback(new PartWriterCallback(stream, (long)CONTENT.length, + cycle.getMetaData(CONTENT_RANGE_STARTBYTE), + cycle.getMetaData(CONTENT_RANGE_ENDBYTE)).setClose(true)); + return response; + } + }; + + ServletWebRequest request = (ServletWebRequest)tester.getRequestCycle().getRequest(); + MockHttpServletRequest httpRequest = (MockHttpServletRequest)request.getContainerRequest(); + httpRequest.setHeader("range", "bytes=30-"); + MockHttpServletResponse httpResponse = new MockHttpServletResponse(httpRequest); + resource.respond(new Attributes(request, new ServletWebResponse(request, httpResponse))); + + assertEquals(416, httpResponse.getStatus()); + assertEquals(0, httpResponse.getBinaryContent().length); + assertTrue(stream.closed); + } + + @Test + void rangeStartingPastTheEndWritesNothingWhenSkipFallsShort() throws IOException + { + ByteArrayResponse response = new ByteArrayResponse(); + + new PartWriterCallback(new TrackingInputStream(1), (long)CONTENT.length, 30L, -1L) + .writeData(new Attributes(tester.getRequestCycle().getRequest(), response)); + + assertEquals(0, response.getBytes().length); + } + + private static class TrackingInputStream extends ByteArrayInputStream + { + private final int maxSkip; + + private boolean closed; + + private TrackingInputStream(int maxSkip) + { + super(CONTENT); + this.maxSkip = maxSkip; + } + + @Override + public synchronized long skip(long n) + { + return super.skip(Math.min(n, maxSkip)); + } + + @Override + public void close() + { + closed = true; + } + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/AbstractResource.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/AbstractResource.java index 91464131df..e2103e19d3 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/resource/AbstractResource.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/AbstractResource.java @@ -57,13 +57,20 @@ public abstract class AbstractResource implements IResource /** header values that are managed internally and must not be set directly */ public static final Set<String> INTERNAL_HEADERS; - /** The meta data key of the content range start byte **/ + /** + * The meta data key of the content range start byte. A negative value stands for a suffix + * range, the last that many bytes of the content. A value at or past the end of the content + * makes the range unsatisfiable. + */ public static final MetaDataKey<Long> CONTENT_RANGE_STARTBYTE = new MetaDataKey<>() { private static final long serialVersionUID = 1L; }; - /** The meta data key of the content range end byte **/ + /** + * The meta data key of the content range end byte, -1 if the range extends to the end of the + * content. + */ public static final MetaDataKey<Long> CONTENT_RANGE_ENDBYTE = new MetaDataKey<>() { private static final long serialVersionUID = 1L; @@ -744,8 +751,25 @@ public abstract class AbstractResource implements IResource String startByteString = rangeParts[0]; String endByteString = rangeParts[1]; - long startbyte = !Strings.isEmpty(startByteString) ? Long.parseLong(startByteString) : 0; - long endbyte = !Strings.isEmpty(endByteString) ? Long.parseLong(endByteString) : -1; + Long startbyte = null; + Long endbyte = null; + if (!Strings.isEmpty(startByteString)) + { + long first = Long.parseLong(startByteString); + long last = !Strings.isEmpty(endByteString) ? Long.parseLong(endByteString) : -1; + if (last == -1 || last >= first) + { + startbyte = first; + endbyte = last; + } + } + else if (!Strings.isEmpty(endByteString)) + { + long suffixLength = Long.parseLong(endByteString); + // a zero-length suffix selects no bytes, just like a range starting past the end + startbyte = suffixLength > 0 ? -suffixLength : Long.MAX_VALUE; + endbyte = -1L; + } // Make the content range information available for the whole request cycle RequestCycle requestCycle = RequestCycle.get(); @@ -850,9 +874,9 @@ public abstract class AbstractResource implements IResource else { // content length has to be set otherwise the content range header can not be - // calculated - accept range must be set to bytes - others are not supported at the - // moment - if (contentLength != -1 && ContentRangeType.BYTES.equals(acceptRange)) + // calculated, and empty content has no part to describe - accept range must be set + // to bytes - others are not supported at the moment + if (contentLength > 0 && ContentRangeType.BYTES.equals(acceptRange)) { contentRangeApplied = setResponseContentRangeHeaderFields(webResponse, attributes, contentLength); @@ -881,7 +905,9 @@ public abstract class AbstractResource implements IResource } /** - * Sets the content range header fields to the given web response + * Sets the content range header fields to the given web response: status 206 with the + * requested range, a suffix range resolved and the last byte limited to the end of the content, + * or status 416 if the range starts at or past the end of the content. * * @param webResponse * the web response to apply the content range information to @@ -902,20 +928,27 @@ public abstract class AbstractResource implements IResource if (startbyte != null && endbyte != null) { - // if end byte hasn't been set - if (endbyte == -1) + long first = firstBytePosition(startbyte, contentLength); + if (first >= contentLength) { - endbyte = contentLength - 1; + webResponse.setStatus(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); + webResponse.setContentRange( + ContentRangeType.BYTES.getTypeName() + " */" + contentLength); + } + else + { + long last = endbyte == -1 ? contentLength - 1 + : Math.min(endbyte, contentLength - 1); + + // Change the status code to 206 partial content + webResponse.setStatus(206); + // currently only bytes are supported. + webResponse.setContentRange(ContentRangeType.BYTES.getTypeName() + " " + + first + '-' + last + '/' + contentLength); + // WARNING - DO NOT SET THE CONTENT LENGTH, even if it is calculated right - + // SAFARI / CHROME are causing issues otherwise! + // webResponse.setContentLength((last - first) + 1); } - - // Change the status code to 206 partial content - webResponse.setStatus(206); - // currently only bytes are supported. - webResponse.setContentRange(ContentRangeType.BYTES.getTypeName() + " " + startbyte + - '-' + endbyte + '/' + contentLength); - // WARNING - DO NOT SET THE CONTENT LENGTH, even if it is calculated right - - // SAFARI / CHROME are causing issues otherwise! - // webResponse.setContentLength((endbyte - startbyte) + 1); // content range has been applied do not set the content length again! contentRangeApplied = true; @@ -924,6 +957,16 @@ public abstract class AbstractResource implements IResource return contentRangeApplied; } + /** + * Resolves a start byte taken from {@link #CONTENT_RANGE_STARTBYTE} against the length of the + * content, so that a suffix range starts that many bytes before the end, but not before the + * first byte. + */ + static long firstBytePosition(long startbyte, long contentLength) + { + return startbyte < 0 ? Math.max(0, contentLength + startbyte) : startbyte; + } + /** * Callback invoked when resource data needs to be written to response. Subclass needs to * implement the {@link #writeData(org.apache.wicket.request.resource.IResource.Attributes)} diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/PartWriterCallback.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/PartWriterCallback.java index 8e2ce38545..8bdd0f6915 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/resource/PartWriterCallback.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/PartWriterCallback.java @@ -44,13 +44,14 @@ public class PartWriterCallback extends WriteCallback private final InputStream inputStream; /** - * The total length to read if {@link #endbyte} is not specified + * The total length to read if {@link #endbyte} is not specified, and to count a negative + * {@link #startbyte} back from */ private final Long contentLength; /** * The byte to start reading from. If omitted then the input stream will be read from its - * beginning + * beginning. A negative value selects the last that many bytes. */ private Long startbyte; @@ -75,17 +76,19 @@ public class PartWriterCallback extends WriteCallback * Creates a part writer callback.<br> * <br> * Reads a part of the given input stream. If the startbyte parameter is not null the number of - * bytes are skipped till the stream is read. If the endbyte is not null the stream is read till - * endbyte, else to the end of the whole stream. If startbyte and endbyte is null the whole - * stream is copied. + * bytes are skipped till the stream is read; a negative startbyte selects the last that many + * bytes of the stream instead. If the endbyte is not null the stream is read till endbyte, else + * to the end of the whole stream. If startbyte and endbyte is null the whole stream is copied. * * @param inputStream * the input stream to read from * @param contentLength - * content length of the input stream. Ignored if <em>endByte</em> is specified + * content length of the input stream. Ignored if <em>endByte</em> is specified and + * <em>startbyte</em> is not negative * @param startbyte * the start position to read from (if not null the number of bytes are skipped till - * the stream is read) + * the stream is read), or the negated number of bytes to read from the end of the + * stream * @param endbyte * the end position to read to (if not null the stream is going to be read till * endbyte, else to the end of the whole stream) @@ -117,10 +120,12 @@ public class PartWriterCallback extends WriteCallback if (startbyte != null || endbyte != null) { - // skipping the first bytes which are - // requested to be skipped by the client if (startbyte != null) { + startbyte = AbstractResource.firstBytePosition(startbyte, contentLength); + + // skipping the first bytes which are + // requested to be skipped by the client inputStream.skip(startbyte); } else @@ -136,6 +141,8 @@ public class PartWriterCallback extends WriteCallback endbyte = contentLength; } + boolean startsPastTheEnd = contentLength >= 0 && startbyte >= contentLength; + BoundedInputStream boundedInputStream = null; try { @@ -143,7 +150,7 @@ public class PartWriterCallback extends WriteCallback // till the end byte computed by the range between startbyte / endbyte boundedInputStream = BoundedInputStream.builder() .setInputStream(inputStream) - .setMaxCount((endbyte - startbyte) + 1) + .setMaxCount(startsPastTheEnd ? 0 : (endbyte - startbyte) + 1) // The original input stream is going to be closed by the end of the request // so set propagate close to false .setPropagateClose(false)
