Skip to content

Commit

Permalink
Fix range responses
Browse files Browse the repository at this point in the history 
Use a fixed chunk-size in case the request specifies an open range
which would mean to transport the entire file. In this case, the
client gets some smaller chunk as it is already indicating to
understand range responses. If a file is smaller than this chunksize,
it will be a normal response.

Fixes: #1328
  • Loading branch information
@eikek
eikek committed Mar 2, 2024
1 parent 470157f commit 3c09145
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 44 deletions.
4 changes: 3 additions & 1 deletion modules/backend/src/main/scala/sharry/backend/config/FilesConfig.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import cats.syntax.all._

import sharry.common.Ident
import sharry.store.FileStoreConfig
import sharry.common.ByteSize

case class FilesConfig(
defaultStore: Ident,
stores: Map[Ident, FileStoreConfig],
copyFiles: CopyFilesConfig
copyFiles: CopyFilesConfig,
downloadChunkSize: ByteSize
) {

val enabledStores: Map[Ident, FileStoreConfig] =
Expand Down
4 changes: 4 additions & 0 deletions modules/restserver/src/main/resources/reference.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ sharry.restserver {
# How many files to copy in parallel.
parallel = 2
}

# For open range requests, use this amount of data when
# responding.
download-chunk-size = "4M"
}

# Checksums of uploaded files are computed in the background.
Expand Down
99 changes: 59 additions & 40 deletions modules/restserver/src/main/scala/sharry/restserver/routes/ByteResponse.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import sharry.backend.share._
import sharry.common._
import sharry.store.records.RFileMeta

import fs2.Stream
import binny.ByteRange
import org.http4s._
import org.http4s.dsl.Http4sDsl
Expand All @@ -23,36 +24,34 @@ object ByteResponse {
backend: BackendApp[F],
shareId: ShareId,
pass: Option[Password],
chunkSize: ByteSize,
fid: Ident
) =
): F[Response[F]] =
req.headers
.get[Range]
.map(_.ranges.head)
.map(sr => range(dsl, sr, backend, shareId, pass, fid))
.map(sr => range(dsl, req, sr, backend, shareId, pass, chunkSize, fid))
.getOrElse(all(dsl, req, backend, shareId, pass, fid))

def range[F[_]: Sync](
dsl: Http4sDsl[F],
req: Request[F],
sr: Range.SubRange,
backend: BackendApp[F],
shareId: ShareId,
pass: Option[Password],
chunkSize: ByteSize,
fid: Ident
): F[Response[F]] = {
import dsl._

val rangeDef: ByteRange = sr.second
.map(until => ByteRange(sr.first, (until - sr.first + 1).toInt))
.getOrElse {
if (sr.first == 0) ByteRange.All
else ByteRange(sr.first, Int.MaxValue)
}

val rangeDef = makeBinnyByteRange(sr, chunkSize)
(for {
file <- backend.share.loadFile(shareId, fid, pass, rangeDef)
resp <- OptionT.liftF {
if (rangeInvalid(file.fileMeta, sr)) RangeNotSatisfiable()
else partialResponse(dsl, file, sr)
else if (file.fileMeta.length <= chunkSize) allBytes(dsl, req, file)
else partialResponse(dsl, req, file, chunkSize, sr)
}
} yield resp).getOrElseF(NotFound())
}
Expand All @@ -69,27 +68,34 @@ object ByteResponse {

(for {
file <- backend.share.loadFile(shareId, fid, pass, ByteRange.All)
resp <- OptionT.liftF(
etagResponse(dsl, req, file).getOrElseF(
Ok.apply(file.data)
.map(setETag(file.fileMeta))
.map(
_.putHeaders(
`Content-Type`(mediaType(file)),
`Accept-Ranges`.bytes,
`Last-Modified`(timestamp(file)),
`Content-Disposition`("inline", fileNameMap(file)),
fileSizeHeader(file.fileMeta.length)
)
)
)
)
resp <- OptionT.liftF(allBytes(dsl, req, file))
} yield resp).getOrElseF(NotFound())
}

private def setETag[F[_]](fm: RFileMeta)(r: Response[F]): Response[F] =
if (fm.checksum.isEmpty) r
else r.putHeaders(ETag(fm.checksum.toHex))
def allBytes[F[_]: Sync](
dsl: Http4sDsl[F],
req: Request[F],
file: FileRange[F]
): F[Response[F]] = {
import dsl._

val isHead = req.method == Method.HEAD
val data = if (!isHead) file.data else Stream.empty
etagResponse(dsl, req, file).getOrElseF(
Ok(data)
.map(setETag(file.fileMeta))
.map(
_.putHeaders(
`Content-Type`(mediaType(file)),
`Accept-Ranges`.bytes,
`Last-Modified`(timestamp(file)),
`Content-Disposition`("inline", fileNameMap(file)),
`Content-Length`(file.fileMeta.length.bytes),
fileSizeHeader(file.fileMeta.length)
)
)
)
}

private def etagResponse[F[_]: Sync](
dsl: Http4sDsl[F],
Expand All @@ -106,31 +112,44 @@ object ByteResponse {

private def partialResponse[F[_]: Sync](
dsl: Http4sDsl[F],
req: Request[F],
file: FileRange[F],
chunkSize: ByteSize,
range: Range.SubRange
): F[Response[F]] = {
import dsl._
val len = file.fileMeta.length
val respLen = range.second.getOrElse(len.bytes) - range.first + 1
PartialContent(file.data.take(respLen)).map(

val fileLen = file.fileMeta.length
val respLen =
range.second.map(until => until - range.first + 1).getOrElse(chunkSize.bytes)
val respRange =
Range.SubRange(range.first, range.second.getOrElse(range.first + chunkSize.bytes))

val isHead = req.method == Method.HEAD
val data = if (isHead) Stream.empty else file.data.take(respLen.toLong)
PartialContent(data).map(
_.withHeaders(
`Accept-Ranges`.bytes,
`Content-Type`(mediaType(file)),
`Last-Modified`(timestamp(file)),
`Content-Disposition`("inline", fileNameMap(file)),
fileSizeHeader(ByteSize(respLen)),
`Content-Range`(RangeUnit.Bytes, subRangeResp(range, len.bytes), Some(len.bytes))
fileSizeHeader(file.fileMeta.length),
`Content-Range`(RangeUnit.Bytes, respRange, Some(fileLen.bytes))
)
)
}

private def subRangeResp(in: Range.SubRange, length: Long): Range.SubRange =
in match {
case Range.SubRange(n, None) =>
Range.SubRange(n, Some(length))
case Range.SubRange(n, Some(t)) =>
Range.SubRange(n, Some(t))
}
private def makeBinnyByteRange(sr: Range.SubRange, chunkSize: ByteSize): ByteRange =
sr.second
.map(until => ByteRange(sr.first, (until - sr.first + 1).toInt))
.getOrElse {
if (sr.first == 0) ByteRange(0, chunkSize.bytes.toInt)
else ByteRange(sr.first, chunkSize.bytes.toInt)
}

private def setETag[F[_]](fm: RFileMeta)(r: Response[F]): Response[F] =
if (fm.checksum.isEmpty) r
else r.putHeaders(ETag(fm.checksum.toHex))

private def rangeInvalid(file: RFileMeta, range: Range.SubRange): Boolean =
range.first < 0 || range.second.exists(t => t < range.first || t > file.length.bytes)
Expand Down
8 changes: 6 additions & 2 deletions modules/restserver/src/main/scala/sharry/restserver/routes/OpenShareRoutes.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@ object OpenShareRoutes {

case req @ GET -> Root / Ident(id) / "file" / Ident(fid) =>
val pw = SharryPassword(req)
ByteResponse(dsl, req, backend, ShareId.publish(id), pw, fid)
val chunkSize = cfg.backend.files.downloadChunkSize
ByteResponse(dsl, req, backend, ShareId.publish(id), pw, chunkSize, fid)

case req @ HEAD -> Root / Ident(id) / "file" / Ident(fid) =>
val pw = SharryPassword(req)
val chunkSize = cfg.backend.files.downloadChunkSize
ByteResponse(dsl, req, backend, ShareId.publish(id), pw, chunkSize, fid)
}
}

}
8 changes: 7 additions & 1 deletion modules/restserver/src/main/scala/sharry/restserver/routes/ShareRoutes.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,13 @@ object ShareRoutes {

case req @ GET -> Root / Ident(id) / "file" / Ident(fid) =>
val pw = SharryPassword(req)
ByteResponse(dsl, req, backend, ShareId.secured(id, token.account), pw, fid)
val chunkSize = cfg.backend.files.downloadChunkSize
ByteResponse(dsl, req, backend, ShareId.secured(id, token.account), pw, chunkSize, fid)

case req @ HEAD -> Root / Ident(id) / "file" / Ident(fid) =>
val pw = SharryPassword(req)
val chunkSize = cfg.backend.files.downloadChunkSize
ByteResponse(dsl, req, backend, ShareId.secured(id, token.account), pw, chunkSize, fid)

// make it safer by also using the share id
case DELETE -> Root / Ident(_) / "file" / Ident(fid) =>
Expand Down

0 comments on commit 3c09145

Please sign in to comment.