diff --git a/CHANGES.txt b/CHANGES.txt index 27ce525fa..1c92ca586 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,11 @@ 1.0.0 ----- + * Refactor range and add toString (CASSANDRASC-41) + * Fix search in list snapshot endpoint (CASSANDRASC-40) + * Allow Cassandra input validation to be configurable (CASSANDRAC-39) + * Add endpoint to list snapshot files (CASSANDRASC-38) + * Optimize file path builder and have separate handler for streaming file (CASSANDRASC-37) + * Support for ErrorHandler in Sidecar (CASSANDRASC-36) * Allow injecting a LoggerHandler to vertxRouter (CASSANDRASC-34) * Optionally support multiple cassandra instances in Sidecar (CASSANDRASC-33) * Call the start method of CassandraAdaptorDelegate to start periodic health check (CASSANDRASC-32) diff --git a/src/main/java/org/apache/cassandra/sidecar/models/Range.java b/src/main/java/org/apache/cassandra/sidecar/models/Range.java index b0050641a..4f8a0069d 100644 --- a/src/main/java/org/apache/cassandra/sidecar/models/Range.java +++ b/src/main/java/org/apache/cassandra/sidecar/models/Range.java @@ -31,132 +31,130 @@ * Accepted Range formats are start-end, start-, -suffix_length * start-end (start = start index of the range, end = end index of the range, both inclusive) * start- (start = start index of the range, end = end of file) - * -suffix-length (Requested length from end of file) + * -suffix-length (Requested length from end of file. The length should be positive) */ public class Range { - private static final Pattern START_END = Pattern.compile("^(\\d+)-(\\d+)$"); - private static final Pattern PARTIAL = Pattern.compile("^((\\d+)-)$|^(-(\\d+))$"); private static final String RANGE_UNIT = "bytes"; + // matches a. bytes=1-2, b. bytes=1-, c. bytes=-2, d. bytes=-. Need to do another valid for case d. + private static final Pattern RANGE_HEADER = Pattern.compile("^" + RANGE_UNIT + "=(\\d*)-(\\d*)$"); private final long start; private final long end; private final long length; + private static final long BOUND_ABSENT = -1L; - private Range(final long start, final long end) + /** + * Accepted RangeHeader formats are bytes=start-end, bytes=start-, bytes=-suffix_length + */ + public static Range parseHeader(final String header, final long fileSize) { - this.start = start; - this.end = end; - this.length = length(start, end); + if (header == null) + { + return new Range(0, fileSize - 1); + } + return Range.parse(header, fileSize); } - public Range(final long start, final long end, final long length) + public static Range of(final long start, final long end) { - this.start = start; - this.end = end; - this.length = length; + return new Range(start, end); } - private long length(final long start, final long end) + /** + * Accepted string formats "bytes=1453-3563", "bytes=-22344", "bytes=5346-" + * Sample invalid string formats "bytes=8-3", "bytes=-", "bytes=-0", "bytes=a-b" + * + * @param fileSize - passed in to convert partial range into absolute range + */ + private static Range parse(@NotNull String rangeHeader, final long fileSize) { - if (start == 0 && end == Long.MAX_VALUE) // avoid overflow (extra byte) + Matcher m = RANGE_HEADER.matcher(rangeHeader); + if (!m.matches()) { - return Long.MAX_VALUE; + throw invalidRangeHeaderException(rangeHeader); } - return end - start + 1; - } - public long start() - { - return this.start; + long left = parseLong(m.group(1), rangeHeader); + long right = parseLong(m.group(2), rangeHeader); + + if (left == BOUND_ABSENT && right == BOUND_ABSENT) // matching "bytes=-" + { + throw invalidRangeHeaderException(rangeHeader); + } + else if (left == BOUND_ABSENT) // matching "bytes=-1" + { + long len = Math.min(right, fileSize); // correct the range if it exceeds file size. + return new Range(fileSize - len, fileSize - 1); + } + else if (right == BOUND_ABSENT) // matching "bytes=1-" + { + return new Range(left, fileSize - 1); + } + else + { + return new Range(left, right); + } } - public long end() + // return -1 for empty string; return long value otherwise. + // throws IllegalArgumentException for invalid value string + private static long parseLong(String valStr, String rangeHeader) { - return this.end; + if (valStr == null || valStr.isEmpty()) + return BOUND_ABSENT; + + try + { + return Long.parseLong(valStr); + } + catch (NumberFormatException e) + { + throw invalidRangeHeaderException(rangeHeader); + } } - public long length() + private static IllegalArgumentException invalidRangeHeaderException(String rangeHeader) { - return this.length; + return new IllegalArgumentException("Invalid range header: " + rangeHeader + ". " + + "Supported Range formats are bytes=-, bytes=-, bytes=-"); } - public boolean isValidHttpRange() + // An initialized range is always valid; invalid params fail range initialization. + private Range(final long start, final long end) { - return start >= 0 && end >= start && length > 0; + Preconditions.checkArgument(start >= 0, "Range start can not be negative"); + Preconditions.checkArgument(end >= start, "Range does not satisfy boundary requirements"); + this.start = start; + this.end = end; + long len = end - start + 1; // Assign long max if overflows + this.length = len < 0 ? Long.MAX_VALUE : len; } - public Range intersect(@NotNull final Range range) + public long start() { - if (!(start >= range.start() && start <= range.end()) && !(end >= range.start() && end <= range.end()) && - !(range.start() >= start && range.start() <= end) && !(range.end() >= start && range.end() <= end)) - { - throw new RangeException("Range does not overlap"); - } - - return new Range(Math.max(start, range.start()), Math.min(end, range.end())); + return this.start; } - /** - * Accepted string formats "start-end", both ends of the range required to be parsed - * Sample accepted formats "1-2", "232-2355" - */ - private static Range parseAbsolute(@NotNull String range) + public long end() { - Matcher m = START_END.matcher(range); - - if (!m.matches()) - { - throw new IllegalArgumentException("Supported Range formats are -, -, -"); - } - - final long start = Long.parseLong(m.group(1)); - Preconditions.checkArgument(start >= 0, "Range start can not be negative"); - - final long end = Long.parseLong(m.group(2)); - if (end < start) - { - throw new RangeException("Range does not satisfy boundary requirements"); - } - return new Range(start, end); + return this.end; } - public static Range parse(@NotNull String range) + public long length() { - // since file size is not passed, we set it to 0 - return parse(range, 0); + return this.length; } - /** - * Accepted string formats "1453-3563", "-22344", "5346-" - * Sample invalid string formats "8-3", "-", "-0", "a-b" - * - * @param fileSize - passed in to convert partial range into absolute range - */ - public static Range parse(@NotNull String range, final long fileSize) + public Range intersect(@NotNull final Range range) { - Matcher m = PARTIAL.matcher(range); - if (!m.matches()) + long newStart = Math.max(start, range.start()); + long newEnd = Math.min(end, range.end()); + if (newStart > newEnd) { - return parseAbsolute(range); - } - - Preconditions.checkArgument(fileSize > 0, "Reference file size invalid"); - if (range.startsWith("-")) - { - final long length = Long.parseLong(m.group(4)); - if (length <= 0) - { - throw new IllegalArgumentException("Suffix length in " + range + " cannot be less than or equal to 0"); - } - Preconditions.checkArgument(length <= fileSize, "Suffix length exceeds"); - return new Range(fileSize - length, fileSize - 1, length); + throw new RangeException("Range does not overlap"); } - final long start = Long.parseLong(m.group(2)); - Preconditions.checkArgument(start >= 0, "Range start can not be negative"); - Preconditions.checkArgument(start < fileSize, "Range exceeds"); - - return new Range(start, fileSize - 1); + return new Range(newStart, newEnd); } @Override @@ -172,8 +170,8 @@ public boolean equals(Object o) } Range range = (Range) o; return start == range.start && - end == range.end && - length == range.length; + end == range.end && + length == range.length; } @Override @@ -182,19 +180,9 @@ public int hashCode() return Objects.hash(start, end, length); } - /** - * Accepted RangeHeader formats are bytes=start-end, bytes=start-, bytes=-suffix_length - */ - public static Range parseHeader(final String header, final long fileSize) + @Override + public String toString() { - if (header == null) - { - return new Range(0, fileSize - 1, fileSize); - } - if (!header.startsWith(RANGE_UNIT + "=")) - { - throw new UnsupportedOperationException("Unsupported range unit only bytes are allowed"); - } - return Range.parse(header.substring(header.indexOf("=") + 1), fileSize); + return String.format("%s=%d-%d", RANGE_UNIT, start, end); } } diff --git a/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java b/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java index c327d3469..53de99f45 100644 --- a/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java +++ b/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java @@ -198,7 +198,7 @@ private boolean checkRetriesExhausted(Instant startTime) */ private Future parseRangeHeader(String rangeHeader, long fileLength) { - Range fr = new Range(0, fileLength - 1, fileLength); + Range fr = Range.of(0, fileLength - 1); if (rangeHeader == null) return Future.succeededFuture(fr); diff --git a/src/test/java/org/apache/cassandra/sidecar/RangeTest.java b/src/test/java/org/apache/cassandra/sidecar/RangeTest.java index e98392dff..2d301ff15 100644 --- a/src/test/java/org/apache/cassandra/sidecar/RangeTest.java +++ b/src/test/java/org/apache/cassandra/sidecar/RangeTest.java @@ -25,7 +25,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; /** * RangeTest @@ -35,12 +34,23 @@ public class RangeTest @Test public void testValidPartialRange() { - final String rangeHeaderVal = "bytes=2-"; - final Range range = Range.parseHeader(rangeHeaderVal, 5); + String rangeHeaderVal = "bytes=2-"; + Range range = Range.parseHeader(rangeHeaderVal, 5); assertEquals(3, range.length()); assertEquals(2, range.start()); assertEquals(4, range.end()); - assertTrue(range.isValidHttpRange()); + + rangeHeaderVal = "bytes=-100"; + range = Range.parseHeader(rangeHeaderVal, 5); + assertEquals(5, range.length()); + assertEquals(0, range.start()); + assertEquals(4, range.end()); + + rangeHeaderVal = "bytes=-100"; + range = Range.parseHeader(rangeHeaderVal, 200); + assertEquals(100, range.length()); + assertEquals(100, range.start()); + assertEquals(199, range.end()); } @Test @@ -51,40 +61,39 @@ public void testValidFullRange() assertEquals(101, range.length()); assertEquals(0, range.start()); assertEquals(100, range.end()); - assertTrue(range.isValidHttpRange()); } @Test public void testInvalidRangeFormat() { - final String rangeVal = "2344--3432"; + final String rangeHeader = "bytes=2344--3432"; IllegalArgumentException thrownException = assertThrows(IllegalArgumentException.class, () -> { - Range.parse(rangeVal); + Range.parseHeader(rangeHeader, Long.MAX_VALUE); }); - String msg = "Supported Range formats are -, -, -"; + String msg = "Invalid range header: bytes=2344--3432. Supported Range formats are bytes=-, bytes=-, bytes=-"; assertEquals(msg, thrownException.getMessage()); } @Test public void testInvalidSuffixLength() { - final String rangeVal = "-0"; + final String rangeHeader = "bytes=-0"; IllegalArgumentException thrownException = assertThrows(IllegalArgumentException.class, () -> { - Range.parse(rangeVal, Long.MAX_VALUE); + Range.parseHeader(rangeHeader, Long.MAX_VALUE); }); - String msg = "Suffix length in -0 cannot be less than or equal to 0"; + String msg = "Range does not satisfy boundary requirements"; assertEquals(msg, thrownException.getMessage()); } @Test public void testInvalidRangeBoundary() { - final String rangeVal = "9-2"; - RangeException thrownException = assertThrows(RangeException.class, () -> + final String rangeHeader = "bytes=9-2"; + IllegalArgumentException thrownException = assertThrows(IllegalArgumentException.class, () -> { - Range.parse(rangeVal); + Range.parseHeader(rangeHeader, Long.MAX_VALUE); }); String msg = "Range does not satisfy boundary requirements"; assertEquals(msg, thrownException.getMessage()); @@ -94,11 +103,63 @@ public void testInvalidRangeBoundary() public void testWrongRangeUnitUsed() { final String rangeVal = "bits=0-"; - UnsupportedOperationException thrownException = assertThrows(UnsupportedOperationException.class, () -> + IllegalArgumentException thrownException = assertThrows(IllegalArgumentException.class, () -> { Range.parseHeader(rangeVal, 5); }); - String msg = "Unsupported range unit only bytes are allowed"; + String msg = "Invalid range header: bits=0-. Supported Range formats are bytes=-, bytes=-, bytes=-"; + assertEquals(msg, thrownException.getMessage()); + } + + @Test + public void testToString() + { + final String rangeHeaderVal = "bytes=0-100"; + final Range range = Range.parseHeader(rangeHeaderVal, Long.MAX_VALUE); + assertEquals("bytes=0-100", range.toString()); + } + + @Test + public void testInvalidRangeBoundValueInHeader() + { + // the right end of range is larger than long + final String rangeHeader = "bytes=0-1" + Long.MAX_VALUE; + IllegalArgumentException thrownException = assertThrows(IllegalArgumentException.class, () -> + { + Range.parseHeader(rangeHeader, Long.MAX_VALUE); + }); + String msg = "Invalid range header: bytes=0-19223372036854775807. Supported Range formats are bytes=-, bytes=-, bytes=-"; assertEquals(msg, thrownException.getMessage()); } + + @Test + public void testIntersect() { + Range range1, range2, expected; + range1 = Range.of(5, 10); + range2 = Range.of(9, 15); + expected = Range.of(9, 10); + assertEquals(expected, range1.intersect(range2)); + assertEquals(expected, range2.intersect(range1)); + + range1 = Range.of(1, 5); + range2 = Range.of(5, 15); + expected = Range.of(5, 5); + assertEquals(expected, range1.intersect(range2)); + assertEquals(expected, range2.intersect(range1)); + + range1 = Range.of(1, 15); + range2 = Range.of(5, 10); + expected = Range.of(5, 10); + assertEquals(expected, range1.intersect(range2)); + assertEquals(expected, range2.intersect(range1)); + + } + + @Test + public void testRangesDoNotIntersect() { + Range range1 = Range.of(1, 5); + Range range2 = Range.of(9, 15); + + assertThrows(RangeException.class, () -> range1.intersect(range2)); + } } diff --git a/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java b/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java index 91916ed33..2e788c650 100644 --- a/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java +++ b/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java @@ -31,6 +31,7 @@ import com.google.inject.Guice; import com.google.inject.Injector; import com.google.inject.util.Modules; +import io.netty.handler.codec.http.HttpHeaderNames; import io.vertx.core.Vertx; import io.vertx.core.http.HttpServer; import io.vertx.ext.web.client.WebClient; @@ -313,7 +314,10 @@ void testSuffixRangeExceeds(VertxTestContext context) .putHeader("Range", "bytes=-5") .send(context.succeeding(response -> context.verify(() -> { - assertThat(response.statusCode()).isEqualTo(REQUESTED_RANGE_NOT_SATISFIABLE.code()); + assertThat(response.statusCode()).isEqualTo(OK.code()); + assertThat(response.getHeader(HttpHeaderNames.CONTENT_LENGTH.toString())) + .isEqualTo("4") + .describedAs("Server should shrink the range to the file length"); context.completeNow(); }))); }