IReadable.read should document out-of-range errors #2461

New Issue

tahoe-lafs · 2015-07-07T17:57:01Z

daira commented

2015-07-07 17:57:01 +00:00

In interfaces.py, document error behaviour when offset and/or size are outside the bounds of the file.

In interfaces.py, document error behaviour when `offset` and/or `size` are outside the bounds of the file.

tahoe-lafs added the

labels 2015-07-07 17:57:01 +00:00

tahoe-lafs added this to the 1.11.0 milestone 2015-07-07 17:57:01 +00:00

tahoe-lafs modified the milestone from 1.11.0 to 1.10.2

2015-07-17 21:53:53 +00:00

zooko commented

2015-07-21 18:18:46 +00:00

I think the behavior ought to be to return a short result, i.e. return as many bytes as do exist and fall within the requested range, and return the 0-length string if none do. That way clients can issue range read requests without knowing for sure whether the request will start past the end or cross the end, which can allow them to do fewer round trips, maybe, in some cases.

I think the behavior ought to be to return a short result, i.e. return as many bytes as *do* exist and fall within the requested range, and return the 0-length string if none do. That way clients can issue range read requests without knowing for sure whether the request will start past the end or cross the end, which can allow them to do fewer round trips, maybe, in some cases.

daira commented

2015-07-25 00:29:38 +00:00

Replying to zooko:

I think the behavior ought to be to return a short result, i.e. return as many bytes as do exist and fall within the requested range, and return the 0-length string if none do. That way clients can issue range read requests without knowing for sure whether the request will start past the end or cross the end, which can allow them to do fewer round trips, maybe, in some cases.

That's a self-consistent design, but it is not the current design (which is for the web-API to truncate the request as necessary to satisfy HTTP semantics). No extra round trips are ever needed, because the file length is known at the time the web-API makes the read call. Also the design change you suggest is not needed to fix #2459, the bug that motivated this ticket. I am skeptical about doing anything more than necessary to fix that bug in a point release.

Replying to [zooko](/tahoe-lafs/trac-2024-07-25/issues/2461#issuecomment-97813): > I think the behavior ought to be to return a short result, i.e. return as many bytes as *do* exist and fall within the requested range, and return the 0-length string if none do. That way clients can issue range read requests without knowing for sure whether the request will start past the end or cross the end, which can allow them to do fewer round trips, maybe, in some cases. That's a self-consistent design, but it is not the current design (which is for the web-API to truncate the request as necessary to satisfy HTTP semantics). No extra round trips are ever needed, because the file length is known at the time the web-API makes the `read` call. Also the design change you suggest is not needed to fix #2459, the bug that motivated this ticket. I am skeptical about doing anything more than necessary to fix that bug in a point release.

warner commented

2015-07-28 20:05:06 +00:00

I'll update it to express the current behavior: reject out-of-range reads, permit zero-length reads.

The webapi continues to allow arbitrary oversized Range: headers, since web/filenode.py clips these to the filesize before calling read(). It's just internal callers who must meet the no-overrun requirement (and they can always read the whole file, offset=0 size=None, without paying attention to get_size() ahead of time).

I'll update it to express the current behavior: reject out-of-range reads, permit zero-length reads. The webapi continues to allow arbitrary oversized `Range:` headers, since `web/filenode.py` clips these to the filesize before calling `read()`. It's just internal callers who must meet the no-overrun requirement (and they can always read the whole file, `offset=0 size=None`, without paying attention to `get_size()` ahead of time).

warner commented

2015-07-28 20:09:26 +00:00

Fixed in [6252a72]

warner added the

fixed

label 2015-07-28 20:09:26 +00:00

warner closed this issue

2015-07-28 20:09:26 +00:00

daira commented

2015-07-28 21:52:06 +00:00

Reviewed, +1.

Sign in to join this conversation.