From e6407fc6b391e9b1c621c0b6e3aee2dfec1d1b8b Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 2 Oct 2023 21:44:07 +0200 Subject: [PATCH] Fix handling of artifact range requests Artifact range requests in HawkBit were impossible to do efficiently, as the underlying mechanism had no choice but to read the start byte and discard content both before and after. To address this situation, we need to introduce a new method to DbArtifact: getFileInputStream(long, long), which maps to the semantics of a range request. The next step is to adjust FileStreamingUtil to use it for both simple range and multi-part requests. Signed-off-by: Zygmunt Krynicki --- .../repository/ArtifactFilesystem.java | 20 +++++++++++++++++++ .../artifact/repository/model/DbArtifact.java | 9 +++++++++ .../jpa/EncryptionAwareDbArtifact.java | 5 +++++ .../hawkbit/rest/util/FileStreamingUtil.java | 6 ++---- .../rest/util/FileStreamingUtilTest.java | 5 +++++ 5 files changed, 41 insertions(+), 4 deletions(-) diff --git a/hawkbit-artifact-repository-filesystem/src/main/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystem.java b/hawkbit-artifact-repository-filesystem/src/main/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystem.java index 147d3c26de..65d7c72de5 100644 --- a/hawkbit-artifact-repository-filesystem/src/main/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystem.java +++ b/hawkbit-artifact-repository-filesystem/src/main/java/org/eclipse/hawkbit/artifact/repository/ArtifactFilesystem.java @@ -11,13 +11,18 @@ import java.io.BufferedInputStream; import java.io.File; +import java.io.RandomAccessFile; +import java.nio.channels.Channels; import java.io.FileInputStream; import java.io.FileNotFoundException; +import java.io.IOException; import java.io.InputStream; import java.util.Objects; import javax.validation.constraints.NotNull; +import com.google.common.io.ByteStreams; + import org.eclipse.hawkbit.artifact.repository.model.AbstractDbArtifact; import org.eclipse.hawkbit.artifact.repository.model.DbArtifactHash; @@ -47,4 +52,19 @@ public InputStream getFileInputStream() { throw new ArtifactFileNotFoundException(e); } } + + @Override + // suppress warning, this InputStream needs to be closed by the caller, this + // cannot be closed in this method + @SuppressWarnings("squid:S2095") + public InputStream getFileInputStream(long start, long end) { + try { + var f = new RandomAccessFile(file, "r"); + var ch = f.getChannel(); + ch.position(start); + return ByteStreams.limit(Channels.newInputStream(ch), end - start + 1); + } catch (final IOException e) { + throw new ArtifactFileNotFoundException(e); + } + } } diff --git a/hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/model/DbArtifact.java b/hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/model/DbArtifact.java index b6318dfbfa..92e2b4bbd1 100644 --- a/hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/model/DbArtifact.java +++ b/hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/model/DbArtifact.java @@ -43,4 +43,13 @@ public interface DbArtifact { * @return {@link InputStream} to read from artifact. */ InputStream getFileInputStream(); + + /** + * Creates an {@link InputStream} on a single range of this artifact. + * The caller has to take care of closing the stream. + * Repeatable calls open a new {@link InputStream}. + * + * @return {@link InputStream} to read from artifact. + */ + InputStream getFileInputStream(long start, long end); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/EncryptionAwareDbArtifact.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/EncryptionAwareDbArtifact.java index 4f2f74a98c..35ad9022f6 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/EncryptionAwareDbArtifact.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/EncryptionAwareDbArtifact.java @@ -63,4 +63,9 @@ public String getContentType() { public InputStream getFileInputStream() { return decryptionFunction.apply(encryptedDbArtifact.getFileInputStream()); } + + @Override + public InputStream getFileInputStream(long start, long end) { + return decryptionFunction.apply(encryptedDbArtifact.getFileInputStream(start, end)); + } } diff --git a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java index 1a1341d2f4..43d868f03c 100644 --- a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java +++ b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java @@ -285,8 +285,8 @@ private static ResponseEntity handleMultipartRangeRequest(final DbA final ServletOutputStream to = response.getOutputStream(); for (final ByteRange r : ranges) { - try (final InputStream from = artifact.getFileInputStream()) { + try (final InputStream from = artifact.getFileInputStream(r.getStart(), r.getEnd())) { // Add multipart boundary and header fields for every range. to.println(); to.println("--" + ByteRange.MULTIPART_BOUNDARY); @@ -316,7 +316,7 @@ private static ResponseEntity handleStandardRangeRequest(final DbAr response.setContentLengthLong(r.getLength()); response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT); - try (final InputStream from = artifact.getFileInputStream()) { + try (final InputStream from = artifact.getFileInputStream(r.getStart(), r.getEnd())) { final ServletOutputStream to = response.getOutputStream(); copyStreams(from, to, progressListener, r.getStart(), r.getLength(), filename); } catch (final IOException e) { @@ -340,8 +340,6 @@ private static long copyStreams(final InputStream from, final OutputStream to, long total = 0; int progressPercent = 1; - ByteStreams.skipFully(from, start); - long toRead = length; boolean toContinue = true; long shippedSinceLastEvent = 0; diff --git a/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/FileStreamingUtilTest.java b/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/FileStreamingUtilTest.java index 25bf6d13cf..40801a53dd 100644 --- a/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/FileStreamingUtilTest.java +++ b/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/FileStreamingUtilTest.java @@ -69,6 +69,11 @@ public String getContentType() { public InputStream getFileInputStream() { return new ByteArrayInputStream(CONTENT_BYTES); } + + @Override + public InputStream getFileInputStream(long start, long end) { + return new ByteArrayInputStream(CONTENT_BYTES, (int)start, (int)(end-start)); + } }; @Test