Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

416 Error for Fetching Outside Range #193

Closed
ilan-gold opened this issue Jan 12, 2021 · 6 comments
Closed

416 Error for Fetching Outside Range #193

ilan-gold opened this issue Jan 12, 2021 · 6 comments

Comments

@ilan-gold
Copy link
Contributor

ilan-gold commented Jan 12, 2021

Hi @constantinius,

I'm trying to figure out if this is an issue with tiffs or a bug here. It appears that because of the way block sizes work, it is possible to make a request outside the allowed range of the file and this errors out in browsers (but not in node?). Here is an example:

  const tiff = await geotiff.fromUrl('https://vitessce-demo-data.storage.googleapis.com/test-data/VAN0006-LK-2-85-IMS_PosMode_multilayer.ome.tif?token=')
  // 416 out of range error
  const image = await tiff.getImage(60);

I think this happens both when offset + length > fileSize where fileSize is the size of the tiff and when fileSize < current < offset + length where current is from here:

geotiff.js/src/source.js

Lines 132 to 143 in 8ef472f

async fetch(offset, length, immediate = false) {
const top = offset + length;
// calculate what blocks intersect the specified range (offset + length)
// determine what blocks are already stored or beeing requested
const firstBlockOffset = Math.floor(offset / this.blockSize) * this.blockSize;
const allBlockIds = [];
const missingBlockIds = [];
const blockRequests = [];
for (let current = firstBlockOffset; current < top; current += this.blockSize) {
const blockId = Math.floor(current / this.blockSize);

Does this seem possible? I think it can happen because of const firstBlockOffset = Math.floor(offset / this.blockSize) * this.blockSize;

@ilan-gold
Copy link
Contributor Author

I've got a branch here that gets the max byte allowed (i.e file size): https://github.com/geotiffjs/geotiff.js/compare/master...ilan-gold:ilan-gold/fix_outofrange_issue?expand=1

@ilan-gold
Copy link
Contributor Author

As an update @constantinius the node API also returns a 416 here, but it doesn't error out for some reason. Specifically if you run the code above, you will see a request for byte range 45285376-45350912 in both node and browser cases, but only the browser one errors out (both return 416 error codes, though)

@constantinius
Copy link
Member

@ilan-gold

Thanks for reporting this issue!

There seems to be something amiss here. When I get the headers of the fileI get the following response:

$ curl -I 'https://vitessce-demo-data.storage.googleapis.com/test-data/VAN0006-LK-2-85-IMS_PosMode_multilayer.ome.tif?token='
HTTP/2 200
...
x-goog-stored-content-length: 45284549
content-type: image/tiff
...
accept-ranges: bytes
content-length: 45284549
...

The byte range you said was requested (45285376-45350912) is completely above the file size, thus resulting in the HTTP Error 416. So this indicates, that there is something wrong with the block as a whole that is requested.

When the requested byte range at least intersects with the file size, the server shall respond with the intersection, which your server does correctly:

$ curl "https://vitessce-demo-data.storage.googleapis.com/test-data/VAN0006-LK-2-85-IMS_PosMode_multilayer.ome.tif?token=" -i -H "Range: bytes=45284547-45350912"
HTTP/2 206
...
content-range: bytes 45284547-45284548/45284549
content-length: 2

So, I think there is a bug elsewhere. Maybe too many blocks are requested, which are not actually required? Do you have a stack trace of the issue?

Regarding your fix: I'm not too fond of the solution to be honest. The reason is that we actually get the actual file size with every range request we send (what you actually parse, I think). So ideally, we parse it from the response and set it in the source, to never request outside of the bounds of the file.

I'll investigate a bit myself to see what is going on.

@constantinius
Copy link
Member

@ilan-gold

There was indeed a bug when parsing an IFD. Because I only get the address of the IFD from the previous one (and not its size) I have to assume the byte size of the IFD. In this particular image, this assumed IFD size was actually larger than the IFD in question, resulting in one correct block fetch and one that happened to be completely out of the files bounds.

With b9aad79 I introduced a mechanism where the total file size is stored in the BlockedSource when it is reported in the response. This now happens automatically the first time a byte slice is received, without the need of an extra request.

Please test this and let me know if that fixes your issue.

@ilan-gold
Copy link
Contributor Author

@constantinius I'll test it out now but that certainly looks like it does the trick (and a lot cleaner than my solution!).

@ilan-gold
Copy link
Contributor Author

@constantinius Your fix works well, thanks so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants