You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "ax-lothas (via GitHub)" <gi...@apache.org> on 2023/10/12 14:51:20 UTC
[PR] [IO-427] Add TrailerInputStream [commons-io]
ax-lothas opened a new pull request, #497:
URL: https://github.com/apache/commons-io/pull/497
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #497:
URL: https://github.com/apache/commons-io/pull/497#issuecomment-1774904787
If this class is indeed generally useful, can it be reused from our Tailer classes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "ax-lothas (via GitHub)" <gi...@apache.org>.
ax-lothas commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1360292905
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
Review Comment:
Extending `FilterInputStream` or `ProxyInputStream` would save overriding
* `close`, and
* `available`
but would require to override
* `mark`,
* `reset`, and
* `markSupported`.
The current implementation is incompatible with `mark`/`reset` as it doesn't track which bytes are already read and wich ones are new.
So, I don't see the benefit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1367746293
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
Review Comment:
Personally I prefer constructors, especially when as here there's exactly one constructor.
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
+ throws IOException {
+ if (trailerLength < 0) {
+ throw new IllegalArgumentException("Trailer length must be >= 0: " + trailerLength);
+ }
+ this.source = source;
+ this.trailer = trailerLength == 0 ? IOUtils.EMPTY_BYTE_ARRAY : new byte[trailerLength];
+ IOUtils.readFully(this.source, this.trailer);
+ }
+
+ @Override
+ public int read() throws IOException {
+ // Does exactly on source read call.
+ // Copies this.trailer.length bytes if source is not EOF.
+ final int newByte = this.source.read();
+ if (newByte == IOUtils.EOF || this.trailer.length == 0) {
+ return newByte;
+ }
+ final int ret = this.trailer[0];
+ System.arraycopy(this.trailer, 1, this.trailer, 0, this.trailer.length - 1);
+ this.trailer[this.trailer.length - 1] = (byte) newByte;
+ return ret;
+ }
+
+ @Override
+ public int read(final byte[] b, final int off, final int len) throws IOException {
+ // Does at most 2 IOUtils.read calls to source.
+ // Copies at most 2 * this.trailer.length bytes.
+ // All other bytes are directly written to the target buffer.
+ if (off < 0 || len < 0 || b.length - off < len) {
+ throw new IndexOutOfBoundsException();
+ }
+ if (len == 0) {
+ return 0;
+ }
+ final int readIntoBuffer;
+ int read;
+ // fist step: move trailer + read data
+ // overview - b: [---------], t: [1234] --> b: [1234abcde], t: [fghi]
+ if (len <= this.trailer.length) {
+ // 1 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer can fill b, so only read into trailer needed
+ // b: [----], trailer: [123456789] --> b: [1234], trailer: [----56789]
+ System.arraycopy(this.trailer, 0, b, off, len);
+ readIntoBuffer = len;
+ // b: [1234], trailer: [----56789] --> b: [1234], trailer: [56789----]
+ System.arraycopy(this.trailer, len, this.trailer, 0, this.trailer.length - len);
+ // b: [1234], trailer: [56789----] --> b: [1234], trailer: [56789abcd]
+ read = IOUtils.read(this.source, this.trailer, this.trailer.length - len, len);
+ } else {
+ // 1 or 2 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer smaller than b, so need to read into b and trailer
+ // b: [---------], t: [1234] --> b: [1234-----], t: [----]
+ System.arraycopy(this.trailer, 0, b, off, this.trailer.length);
+ // b: [1234-----], t: [----] --> b: [1234abcde], t: [----]
+ read = IOUtils.read(
+ this.source, b, off + this.trailer.length, len - this.trailer.length);
+ readIntoBuffer = this.trailer.length + read;
+ // b: [1234abcde], t: [----] --> b: [1234abcde], t: [fghi]
+ if (read == len - this.trailer.length) { // don't try reading data when stream source EOF
+ read += IOUtils.read(this.source, this.trailer);
+ }
+ }
+ // if less data than requested has been read, the trailer buffer is not full
+ // --> need to fill the trailer with the last bytes from b
+ // (only possible if we reached EOF)
+ // second step: ensure that trailer is completely filled with data
+ // overview - b: [abcdefghi], t: [jk--] --> b: [abcdefg--], t: [hijk]
+ final int underflow = Math.min(len - read, this.trailer.length);
+ if (underflow > 0) {
+ // at most this.trailer.length are copied to fill the trailer buffer
+ if (underflow < this.trailer.length) {
+ // trailer not completely empty, so move data to the end
+ // b: [abcdefghi], t: [jk--] --> b: [abcdefghi], t: [--jk]
+ System.arraycopy(
+ this.trailer, 0, this.trailer, underflow, this.trailer.length - underflow);
+ }
+ // fill trailer with last bytes from b
+ // b: [abcdefghi], t: [--jk] --> b: [abcdefg--], t: [hijk]
+ System.arraycopy(b, off + readIntoBuffer - underflow, this.trailer, 0, underflow);
+ }
+ // IOUtils.read reads as many bytes as possible, so reading 0 bytes means EOF.
+ // Then, we have to mark this.
+ return read == 0 && len != 0 ? IOUtils.EOF : read;
+ }
+
+ @Override
+ public int available() throws IOException {
+ return this.source.available();
+ }
+
+ @Override
+ public void close() throws IOException {
+ try {
+ this.source.close();
+ } finally {
+ super.close();
+ }
+ }
+
+ public int getTrailerLength() {
+ return this.trailer.length;
+ }
+
+ public byte[] copyTrailer() {
Review Comment:
I think one method to getTrailer() should suffice and would be noticeably simpler. I don't expect trailers to be all that long.
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
+ throws IOException {
+ if (trailerLength < 0) {
+ throw new IllegalArgumentException("Trailer length must be >= 0: " + trailerLength);
+ }
+ this.source = source;
+ this.trailer = trailerLength == 0 ? IOUtils.EMPTY_BYTE_ARRAY : new byte[trailerLength];
+ IOUtils.readFully(this.source, this.trailer);
+ }
+
+ @Override
+ public int read() throws IOException {
+ // Does exactly on source read call.
+ // Copies this.trailer.length bytes if source is not EOF.
+ final int newByte = this.source.read();
+ if (newByte == IOUtils.EOF || this.trailer.length == 0) {
+ return newByte;
+ }
+ final int ret = this.trailer[0];
+ System.arraycopy(this.trailer, 1, this.trailer, 0, this.trailer.length - 1);
+ this.trailer[this.trailer.length - 1] = (byte) newByte;
+ return ret;
+ }
+
+ @Override
+ public int read(final byte[] b, final int off, final int len) throws IOException {
+ // Does at most 2 IOUtils.read calls to source.
+ // Copies at most 2 * this.trailer.length bytes.
+ // All other bytes are directly written to the target buffer.
+ if (off < 0 || len < 0 || b.length - off < len) {
+ throw new IndexOutOfBoundsException();
+ }
+ if (len == 0) {
+ return 0;
+ }
+ final int readIntoBuffer;
+ int read;
+ // fist step: move trailer + read data
+ // overview - b: [---------], t: [1234] --> b: [1234abcde], t: [fghi]
+ if (len <= this.trailer.length) {
+ // 1 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer can fill b, so only read into trailer needed
+ // b: [----], trailer: [123456789] --> b: [1234], trailer: [----56789]
+ System.arraycopy(this.trailer, 0, b, off, len);
+ readIntoBuffer = len;
+ // b: [1234], trailer: [----56789] --> b: [1234], trailer: [56789----]
+ System.arraycopy(this.trailer, len, this.trailer, 0, this.trailer.length - len);
+ // b: [1234], trailer: [56789----] --> b: [1234], trailer: [56789abcd]
+ read = IOUtils.read(this.source, this.trailer, this.trailer.length - len, len);
+ } else {
+ // 1 or 2 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer smaller than b, so need to read into b and trailer
+ // b: [---------], t: [1234] --> b: [1234-----], t: [----]
+ System.arraycopy(this.trailer, 0, b, off, this.trailer.length);
+ // b: [1234-----], t: [----] --> b: [1234abcde], t: [----]
+ read = IOUtils.read(
+ this.source, b, off + this.trailer.length, len - this.trailer.length);
+ readIntoBuffer = this.trailer.length + read;
+ // b: [1234abcde], t: [----] --> b: [1234abcde], t: [fghi]
+ if (read == len - this.trailer.length) { // don't try reading data when stream source EOF
+ read += IOUtils.read(this.source, this.trailer);
+ }
+ }
+ // if less data than requested has been read, the trailer buffer is not full
+ // --> need to fill the trailer with the last bytes from b
+ // (only possible if we reached EOF)
+ // second step: ensure that trailer is completely filled with data
+ // overview - b: [abcdefghi], t: [jk--] --> b: [abcdefg--], t: [hijk]
+ final int underflow = Math.min(len - read, this.trailer.length);
+ if (underflow > 0) {
+ // at most this.trailer.length are copied to fill the trailer buffer
+ if (underflow < this.trailer.length) {
+ // trailer not completely empty, so move data to the end
+ // b: [abcdefghi], t: [jk--] --> b: [abcdefghi], t: [--jk]
+ System.arraycopy(
+ this.trailer, 0, this.trailer, underflow, this.trailer.length - underflow);
+ }
+ // fill trailer with last bytes from b
+ // b: [abcdefghi], t: [--jk] --> b: [abcdefg--], t: [hijk]
+ System.arraycopy(b, off + readIntoBuffer - underflow, this.trailer, 0, underflow);
+ }
+ // IOUtils.read reads as many bytes as possible, so reading 0 bytes means EOF.
+ // Then, we have to mark this.
+ return read == 0 && len != 0 ? IOUtils.EOF : read;
+ }
+
+ @Override
+ public int available() throws IOException {
+ return this.source.available();
Review Comment:
shouldn't this subtract the bytes in the trailer?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1368439414
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ *
+ * <p>
+ * No mark/reset support.
+ * </p>
+ *
+ * <p>
+ * Not thread-safe. If accessed by multiple threads concurrently, external synchronization is
+ * necessary.
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ // The current implementation is incompatible with mark/reset as it doesn't track which bytes are
+ // already read and which ones are new. This tracking would be necessary to not overwrite the
+ // trailer with earlier bytes in the source stream. Remember that the trailer is not meant to
+ // contain the last read bytes but the last bytes in the stream (which differs when using reset
+ // to jump to an earlier position of the source stream).
Review Comment:
The comment needs to explain the choice of super-class. This was done well in the PR, but this summary is a bit too terse.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on PR #497:
URL: https://github.com/apache/commons-io/pull/497#issuecomment-1776214202
Whilst the class seems to work well, I'm not convinced that there is a sufficient need for it.
Every extra class adds to the maintenance load, so there needs to be a demonstrated to make it worth the extra effort.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "ax-lothas (via GitHub)" <gi...@apache.org>.
ax-lothas commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1368275746
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
+ throws IOException {
+ if (trailerLength < 0) {
+ throw new IllegalArgumentException("Trailer length must be >= 0: " + trailerLength);
+ }
+ this.source = source;
+ this.trailer = trailerLength == 0 ? IOUtils.EMPTY_BYTE_ARRAY : new byte[trailerLength];
+ IOUtils.readFully(this.source, this.trailer);
+ }
+
+ @Override
+ public int read() throws IOException {
+ // Does exactly on source read call.
+ // Copies this.trailer.length bytes if source is not EOF.
+ final int newByte = this.source.read();
+ if (newByte == IOUtils.EOF || this.trailer.length == 0) {
+ return newByte;
+ }
+ final int ret = this.trailer[0];
+ System.arraycopy(this.trailer, 1, this.trailer, 0, this.trailer.length - 1);
+ this.trailer[this.trailer.length - 1] = (byte) newByte;
+ return ret;
+ }
+
+ @Override
+ public int read(final byte[] b, final int off, final int len) throws IOException {
+ // Does at most 2 IOUtils.read calls to source.
+ // Copies at most 2 * this.trailer.length bytes.
+ // All other bytes are directly written to the target buffer.
+ if (off < 0 || len < 0 || b.length - off < len) {
+ throw new IndexOutOfBoundsException();
+ }
+ if (len == 0) {
+ return 0;
+ }
+ final int readIntoBuffer;
+ int read;
+ // fist step: move trailer + read data
+ // overview - b: [---------], t: [1234] --> b: [1234abcde], t: [fghi]
+ if (len <= this.trailer.length) {
+ // 1 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer can fill b, so only read into trailer needed
+ // b: [----], trailer: [123456789] --> b: [1234], trailer: [----56789]
+ System.arraycopy(this.trailer, 0, b, off, len);
+ readIntoBuffer = len;
+ // b: [1234], trailer: [----56789] --> b: [1234], trailer: [56789----]
+ System.arraycopy(this.trailer, len, this.trailer, 0, this.trailer.length - len);
+ // b: [1234], trailer: [56789----] --> b: [1234], trailer: [56789abcd]
+ read = IOUtils.read(this.source, this.trailer, this.trailer.length - len, len);
+ } else {
+ // 1 or 2 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer smaller than b, so need to read into b and trailer
+ // b: [---------], t: [1234] --> b: [1234-----], t: [----]
+ System.arraycopy(this.trailer, 0, b, off, this.trailer.length);
+ // b: [1234-----], t: [----] --> b: [1234abcde], t: [----]
+ read = IOUtils.read(
+ this.source, b, off + this.trailer.length, len - this.trailer.length);
+ readIntoBuffer = this.trailer.length + read;
+ // b: [1234abcde], t: [----] --> b: [1234abcde], t: [fghi]
+ if (read == len - this.trailer.length) { // don't try reading data when stream source EOF
+ read += IOUtils.read(this.source, this.trailer);
+ }
+ }
+ // if less data than requested has been read, the trailer buffer is not full
+ // --> need to fill the trailer with the last bytes from b
+ // (only possible if we reached EOF)
+ // second step: ensure that trailer is completely filled with data
+ // overview - b: [abcdefghi], t: [jk--] --> b: [abcdefg--], t: [hijk]
+ final int underflow = Math.min(len - read, this.trailer.length);
+ if (underflow > 0) {
+ // at most this.trailer.length are copied to fill the trailer buffer
+ if (underflow < this.trailer.length) {
+ // trailer not completely empty, so move data to the end
+ // b: [abcdefghi], t: [jk--] --> b: [abcdefghi], t: [--jk]
+ System.arraycopy(
+ this.trailer, 0, this.trailer, underflow, this.trailer.length - underflow);
+ }
+ // fill trailer with last bytes from b
+ // b: [abcdefghi], t: [--jk] --> b: [abcdefg--], t: [hijk]
+ System.arraycopy(b, off + readIntoBuffer - underflow, this.trailer, 0, underflow);
+ }
+ // IOUtils.read reads as many bytes as possible, so reading 0 bytes means EOF.
+ // Then, we have to mark this.
+ return read == 0 && len != 0 ? IOUtils.EOF : read;
+ }
+
+ @Override
+ public int available() throws IOException {
+ return this.source.available();
+ }
+
+ @Override
+ public void close() throws IOException {
+ try {
+ this.source.close();
+ } finally {
+ super.close();
+ }
+ }
+
+ public int getTrailerLength() {
+ return this.trailer.length;
+ }
+
+ public byte[] copyTrailer() {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1368460508
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
Review Comment:
There does not need to be a default if none makes sense, an exception can be thrown at build time or in the private ctor for nonsensical values.
If experience has taught us anything here, it's that there will be constructor creep in the future, so please use a builder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "ax-lothas (via GitHub)" <gi...@apache.org>.
ax-lothas commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1368246334
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
+ throws IOException {
+ if (trailerLength < 0) {
+ throw new IllegalArgumentException("Trailer length must be >= 0: " + trailerLength);
+ }
+ this.source = source;
+ this.trailer = trailerLength == 0 ? IOUtils.EMPTY_BYTE_ARRAY : new byte[trailerLength];
+ IOUtils.readFully(this.source, this.trailer);
+ }
+
+ @Override
+ public int read() throws IOException {
+ // Does exactly on source read call.
+ // Copies this.trailer.length bytes if source is not EOF.
+ final int newByte = this.source.read();
+ if (newByte == IOUtils.EOF || this.trailer.length == 0) {
+ return newByte;
+ }
+ final int ret = this.trailer[0];
+ System.arraycopy(this.trailer, 1, this.trailer, 0, this.trailer.length - 1);
+ this.trailer[this.trailer.length - 1] = (byte) newByte;
+ return ret;
+ }
+
+ @Override
+ public int read(final byte[] b, final int off, final int len) throws IOException {
+ // Does at most 2 IOUtils.read calls to source.
+ // Copies at most 2 * this.trailer.length bytes.
+ // All other bytes are directly written to the target buffer.
+ if (off < 0 || len < 0 || b.length - off < len) {
+ throw new IndexOutOfBoundsException();
+ }
+ if (len == 0) {
+ return 0;
+ }
+ final int readIntoBuffer;
+ int read;
+ // fist step: move trailer + read data
+ // overview - b: [---------], t: [1234] --> b: [1234abcde], t: [fghi]
+ if (len <= this.trailer.length) {
+ // 1 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer can fill b, so only read into trailer needed
+ // b: [----], trailer: [123456789] --> b: [1234], trailer: [----56789]
+ System.arraycopy(this.trailer, 0, b, off, len);
+ readIntoBuffer = len;
+ // b: [1234], trailer: [----56789] --> b: [1234], trailer: [56789----]
+ System.arraycopy(this.trailer, len, this.trailer, 0, this.trailer.length - len);
+ // b: [1234], trailer: [56789----] --> b: [1234], trailer: [56789abcd]
+ read = IOUtils.read(this.source, this.trailer, this.trailer.length - len, len);
+ } else {
+ // 1 or 2 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer smaller than b, so need to read into b and trailer
+ // b: [---------], t: [1234] --> b: [1234-----], t: [----]
+ System.arraycopy(this.trailer, 0, b, off, this.trailer.length);
+ // b: [1234-----], t: [----] --> b: [1234abcde], t: [----]
+ read = IOUtils.read(
+ this.source, b, off + this.trailer.length, len - this.trailer.length);
+ readIntoBuffer = this.trailer.length + read;
+ // b: [1234abcde], t: [----] --> b: [1234abcde], t: [fghi]
+ if (read == len - this.trailer.length) { // don't try reading data when stream source EOF
+ read += IOUtils.read(this.source, this.trailer);
+ }
+ }
+ // if less data than requested has been read, the trailer buffer is not full
+ // --> need to fill the trailer with the last bytes from b
+ // (only possible if we reached EOF)
+ // second step: ensure that trailer is completely filled with data
+ // overview - b: [abcdefghi], t: [jk--] --> b: [abcdefg--], t: [hijk]
+ final int underflow = Math.min(len - read, this.trailer.length);
+ if (underflow > 0) {
+ // at most this.trailer.length are copied to fill the trailer buffer
+ if (underflow < this.trailer.length) {
+ // trailer not completely empty, so move data to the end
+ // b: [abcdefghi], t: [jk--] --> b: [abcdefghi], t: [--jk]
+ System.arraycopy(
+ this.trailer, 0, this.trailer, underflow, this.trailer.length - underflow);
+ }
+ // fill trailer with last bytes from b
+ // b: [abcdefghi], t: [--jk] --> b: [abcdefg--], t: [hijk]
+ System.arraycopy(b, off + readIntoBuffer - underflow, this.trailer, 0, underflow);
+ }
+ // IOUtils.read reads as many bytes as possible, so reading 0 bytes means EOF.
+ // Then, we have to mark this.
+ return read == 0 && len != 0 ? IOUtils.EOF : read;
+ }
+
+ @Override
+ public int available() throws IOException {
+ return this.source.available();
Review Comment:
The trailer is filled in the constructor. So every single byte read after the constructor finished causes exactly one byte to return to the caller of the `read` methods. Think about it as shift register.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #497:
URL: https://github.com/apache/commons-io/pull/497#issuecomment-1777190319
> Whilst the class seems to work well, I'm not convinced that there is a sufficient need for it.
>
> Every extra class adds to the maintenance load, so there needs to be a demonstrated to make it worth the extra effort.
Right, I agree, and as I mentioned earlier: Could this be reused as is within IO itself (Tailer class) and from Tika which provides a similar class?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1359377058
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
Review Comment:
This class should extend `java.io.FilterInputStream` or `org.apache.commons.io.input.ProxyInputStream`.
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
Review Comment:
Use the Builder pattern to avoid constructor creep. For example `org.apache.commons.io.input.BOMInputStream.Builder`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "ax-lothas (via GitHub)" <gi...@apache.org>.
ax-lothas commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1360294274
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
Review Comment:
What should be the default value of `trailerLength`? `0` is rather useless, so both arguments have to be provided anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1368317639
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
Review Comment:
This is an important design decision which needs to be documented in the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1368468564
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ *
+ * <p>
+ * No mark/reset support.
+ * </p>
+ *
+ * <p>
+ * Not thread-safe. If accessed by multiple threads concurrently, external synchronization is
+ * necessary.
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ // Extending FilterInputStream or ProxyInputStream would save overriding
+ // * close, and
+ // * available
+ // but would require to override
+ // * mark,
+ // * reset, and
+ // * markSupported.
+ // So, there is no benefit in extending FilterInputStream or ProxyInputStream over InputStream
+ // as mark/reset is not supported by this implementation.
+
+ // The current implementation is incompatible with mark/reset as it doesn't track which bytes are
+ // already read and which ones are new. This tracking would be necessary to not overwrite the
+ // trailer with earlier bytes in the source stream. Remember that the trailer is not meant to
+ // contain the last read bytes but the last bytes in the stream (which differs when using reset
+ // to jump to an earlier position of the source stream).
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
+ throws IOException {
+ if (trailerLength < 0) {
Review Comment:
I don't see the point of allowing a zero-length trailer
Also, it probably make sense to have an upper limit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IO-427] Add TrailerInputStream [commons-io]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #497:
URL: https://github.com/apache/commons-io/pull/497#discussion_r1368489551
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
Review Comment:
getTrailer
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ *
+ * <p>
+ * No mark/reset support.
+ * </p>
+ *
+ * <p>
+ * Not thread-safe. If accessed by multiple threads concurrently, external synchronization is
+ * necessary.
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ // Extending FilterInputStream or ProxyInputStream would save overriding
+ // * close, and
+ // * available
+ // but would require to override
+ // * mark,
+ // * reset, and
+ // * markSupported.
+ // So, there is no benefit in extending FilterInputStream or ProxyInputStream over InputStream
+ // as mark/reset is not supported by this implementation.
+
+ // The current implementation is incompatible with mark/reset as it doesn't track which bytes are
+ // already read and which ones are new. This tracking would be necessary to not overwrite the
+ // trailer with earlier bytes in the source stream. Remember that the trailer is not meant to
+ // contain the last read bytes but the last bytes in the stream (which differs when using reset
+ // to jump to an earlier position of the source stream).
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
+ throws IOException {
+ if (trailerLength < 0) {
+ throw new IllegalArgumentException("Trailer length must be >= 0: " + trailerLength);
+ }
+ this.source = source;
+ this.trailer = trailerLength == 0 ? IOUtils.EMPTY_BYTE_ARRAY : new byte[trailerLength];
+ IOUtils.readFully(this.source, this.trailer);
+ }
+
+ @Override
+ public int read() throws IOException {
+ // Does exactly on source read call.
+ // Copies this.trailer.length bytes if source is not EOF.
+ final int newByte = this.source.read();
+ if (newByte == IOUtils.EOF || this.trailer.length == 0) {
+ return newByte;
+ }
+ final int ret = this.trailer[0];
+ System.arraycopy(this.trailer, 1, this.trailer, 0, this.trailer.length - 1);
+ this.trailer[this.trailer.length - 1] = (byte) newByte;
+ return ret;
+ }
+
+ @Override
+ public int read(final byte[] b, final int off, final int len) throws IOException {
+ // Does at most 2 IOUtils.read calls to source.
+ // Copies at most 2 * this.trailer.length bytes.
+ // All other bytes are directly written to the target buffer.
+ if (off < 0 || len < 0 || b.length - off < len) {
+ throw new IndexOutOfBoundsException();
+ }
+ if (len == 0) {
+ return 0;
+ }
+ final int readIntoBuffer;
+ int read;
+ // fist step: move trailer + read data
+ // overview - b: [---------], t: [1234] --> b: [1234abcde], t: [fghi]
+ if (len <= this.trailer.length) {
+ // 1 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer can fill b, so only read into trailer needed
+ // b: [----], trailer: [123456789] --> b: [1234], trailer: [----56789]
+ System.arraycopy(this.trailer, 0, b, off, len);
+ readIntoBuffer = len;
+ // b: [1234], trailer: [----56789] --> b: [1234], trailer: [56789----]
+ System.arraycopy(this.trailer, len, this.trailer, 0, this.trailer.length - len);
+ // b: [1234], trailer: [56789----] --> b: [1234], trailer: [56789abcd]
+ read = IOUtils.read(this.source, this.trailer, this.trailer.length - len, len);
+ } else {
+ // 1 or 2 IOUtils.read calls to source, copies this.trailer.length bytes
+ // trailer smaller than b, so need to read into b and trailer
+ // b: [---------], t: [1234] --> b: [1234-----], t: [----]
+ System.arraycopy(this.trailer, 0, b, off, this.trailer.length);
+ // b: [1234-----], t: [----] --> b: [1234abcde], t: [----]
+ read = IOUtils.read(
+ this.source, b, off + this.trailer.length, len - this.trailer.length);
+ readIntoBuffer = this.trailer.length + read;
+ // b: [1234abcde], t: [----] --> b: [1234abcde], t: [fghi]
+ if (read == len - this.trailer.length) { // don't try reading data when stream source EOF
+ read += IOUtils.read(this.source, this.trailer);
+ }
+ }
+ // if less data than requested has been read, the trailer buffer is not full
+ // --> need to fill the trailer with the last bytes from b
+ // (only possible if we reached EOF)
+ // second step: ensure that trailer is completely filled with data
+ // overview - b: [abcdefghi], t: [jk--] --> b: [abcdefg--], t: [hijk]
+ final int underflow = Math.min(len - read, this.trailer.length);
+ if (underflow > 0) {
+ // at most this.trailer.length are copied to fill the trailer buffer
+ if (underflow < this.trailer.length) {
+ // trailer not completely empty, so move data to the end
+ // b: [abcdefghi], t: [jk--] --> b: [abcdefghi], t: [--jk]
+ System.arraycopy(
+ this.trailer, 0, this.trailer, underflow, this.trailer.length - underflow);
+ }
+ // fill trailer with last bytes from b
+ // b: [abcdefghi], t: [--jk] --> b: [abcdefg--], t: [hijk]
+ System.arraycopy(b, off + readIntoBuffer - underflow, this.trailer, 0, underflow);
+ }
+ // IOUtils.read reads as many bytes as possible, so reading 0 bytes means EOF.
+ // Then, we have to mark this.
+ return read == 0 && len != 0 ? IOUtils.EOF : read;
+ }
+
+ @Override
+ public int available() throws IOException {
+ return this.source.available();
+ }
+
+ @Override
+ public void close() throws IOException {
+ try {
+ this.source.close();
+ } finally {
+ super.close();
+ }
+ }
+
+ public int getTrailerLength() {
+ return this.trailer.length;
+ }
+
+ public byte[] copyTrailer() {
Review Comment:
copyTrailer --> getTrailer
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
+ * @throws IOException initializing the trailer buffer failed.
+ */
+ public TrailerInputStream(final InputStream source, final int trailerLength)
Review Comment:
IMHO builders are vastly overused. They're not native to Java's design, and constructors are more natural. There's a place for builders, but they shouldn't be the default.
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
Review Comment:
That seems wonky. I'd almost prefer iot to throw IllegalStateException unitl it has the real trailer
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ *
+ * <p>
+ * No mark/reset support.
+ * </p>
+ *
+ * <p>
+ * Not thread-safe. If accessed by multiple threads concurrently, external synchronization is
+ * necessary.
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ // Extending FilterInputStream or ProxyInputStream would save overriding
+ // * close, and
+ // * available
+ // but would require to override
+ // * mark,
+ // * reset, and
+ // * markSupported.
+ // So, there is no benefit in extending FilterInputStream or ProxyInputStream over InputStream
+ // as mark/reset is not supported by this implementation.
+
+ // The current implementation is incompatible with mark/reset as it doesn't track which bytes are
+ // already read and which ones are new. This tracking would be necessary to not overwrite the
+ // trailer with earlier bytes in the source stream. Remember that the trailer is not meant to
+ // contain the last read bytes but the last bytes in the stream (which differs when using reset
+ // to jump to an earlier position of the source stream).
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
Review Comment:
has to be --> is
##########
src/main/java/org/apache/commons/io/input/TrailerInputStream.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed 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.commons.io.input;
+
+import java.io.IOException;
+import java.io.InputStream;
+import org.apache.commons.io.IOUtils;
+
+/**
+ * Reads the underlying input stream while holding back the trailer.
+ *
+ * <p>
+ * "Normal" read calls read the underlying stream except the last few bytes (the trailer). The
+ * trailer is updated with each read call. The trailer can be gotten by one of the copyTrailer
+ * overloads.
+ * </p>
+ *
+ * <p>
+ * It is safe to fetch the trailer at any time but the trailer will change with each read call
+ * until the underlying stream is EOF.
+ * </p>
+ *
+ * <p>
+ * Useful, e.g., for handling checksums: payload is followed by a fixed size hash, so while
+ * streaming the payload the trailer finally contains the expected hash (this example needs
+ * extra caution to revert actions when the final checksum match fails).
+ * </p>
+ *
+ * <p>
+ * No mark/reset support.
+ * </p>
+ *
+ * <p>
+ * Not thread-safe. If accessed by multiple threads concurrently, external synchronization is
+ * necessary.
+ * </p>
+ */
+public final class TrailerInputStream extends InputStream {
+
+ // Extending FilterInputStream or ProxyInputStream would save overriding
+ // * close, and
+ // * available
+ // but would require to override
+ // * mark,
+ // * reset, and
+ // * markSupported.
+ // So, there is no benefit in extending FilterInputStream or ProxyInputStream over InputStream
+ // as mark/reset is not supported by this implementation.
+
+ // The current implementation is incompatible with mark/reset as it doesn't track which bytes are
+ // already read and which ones are new. This tracking would be necessary to not overwrite the
+ // trailer with earlier bytes in the source stream. Remember that the trailer is not meant to
+ // contain the last read bytes but the last bytes in the stream (which differs when using reset
+ // to jump to an earlier position of the source stream).
+
+ private final InputStream source;
+ /**
+ * Invariant: After every method call which exited without exception, the trailer has to be
+ * completely filled.
+ */
+ private final byte[] trailer;
+
+ /**
+ * Constructs the TrailerInputStream and initializes the trailer buffer.
+ *
+ * <p>
+ * Reads exactly {@code trailerLength} bytes from {@code source}.
+ * </p>
+ *
+ * @param source underlying stream from which is read.
+ * @param trailerLength the length of the trailer which is hold back (must be >= 0).
Review Comment:
held back
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org