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

error loading COG with response status 200 instead of 206 #207

Open
jcphill opened this issue Mar 16, 2021 · 2 comments
Open

error loading COG with response status 200 instead of 206 #207

jcphill opened this issue Mar 16, 2021 · 2 comments

Comments

@jcphill
Copy link
Contributor

jcphill commented Mar 16, 2021

This section of fetch() in source.js starting at line 188 (https://github.com/GeoTIFF/geotiff.js/blob/e2bab6c150035ac277aaedc4efb928813d0e06a3/src/source.js#L188) assumes that response.offset is equal to the requested offset (group[0] * this.blockSize), but that is only true if the fetch returned 206 with the requested range rather than 200 and the entire file (in which case response.offset is 0).

    for (let i = 0; i < group.length; ++i) {
      const id = group[i];
      this.blockRequests.set(id, (async () => {
        const response = await request;
        const o = i * this.blockSize;
        const t = Math.min(o + this.blockSize, response.data.byteLength);
        const data = response.data.slice(o, t);
        this.blockRequests.delete(id);
        this.blocks.set(id, {
          data,
          offset: response.offset + o,
          length: data.byteLength,
          top: response.offset + t,
        });
      })());
    }

The fix (untested) is: const o = id * this.blockSize - response.offset;

The bug is exhibited as a RangeError when loading from an object URL created from a File in Firefox on a Mac, but would also happen on any server that (legally) ignored a range request.

@constantinius
Copy link
Member

Hi @jcphill

Thanks for reporting that! This is actually a rather problematic topic, as TIFF files have the tendency to be rather large and more often than not not actually fitting in memory and it is likely better to not actually accept 200 responses but aborting the request instead.

There is, however, an extensive rework of the whole sources in the works: #94

I think such responses are handled better there. Maybe you can try that branch?

@jcphill
Copy link
Contributor Author

jcphill commented Mar 17, 2021

Thank you for your reply.

The logic is correct in the pull request at

const o = blockOffset - response.offset;

I'm glad to see that there is an allowFullFile option in the pull request to accept 200 responses.

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