From 1119a6bb08b21a922d64d958ab3b7ba9fd309fc3 Mon Sep 17 00:00:00 2001 From: Piotr Jurczynski <619650+pjurczynski@users.noreply.github.com> Date: Thu, 24 Jul 2025 16:50:02 +0200 Subject: [PATCH 1/2] Fix incorrect slice read in blocks targets a TypeError "TypeError: Cannot read properties of undefined (reading 'offset')" that was happening when: - a slice read was equal to a block size - reading last part of the data The fix removes the "one-off" bug: - top is the first byte of the next block that we don't want to read. - The last block we want to read (blockIdHigh) is found by knowing the last byte we want to read and divide it by the block size. - Moved blockedSource declaration inside each test for better isolation. (BlockedSource is stateful) --- src/source/blockedsource.js | 4 +- test/geotiff.spec.js | 84 ++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/source/blockedsource.js b/src/source/blockedsource.js index 5630efb2..1bb0cb58 100644 --- a/src/source/blockedsource.js +++ b/src/source/blockedsource.js @@ -1,6 +1,6 @@ import QuickLRU from 'quick-lru'; -import { BaseSource } from './basesource.js'; import { AbortError, AggregateError, wait, zip } from '../utils.js'; +import { BaseSource } from './basesource.js'; class Block { /** @@ -262,7 +262,7 @@ export class BlockedSource extends BaseSource { top = Math.min(this.fileSize, top); } const blockIdLow = Math.floor(slice.offset / this.blockSize); - const blockIdHigh = Math.floor(top / this.blockSize); + const blockIdHigh = Math.floor((top - 1) / this.blockSize); const sliceData = new ArrayBuffer(slice.length); const sliceView = new Uint8Array(sliceData); diff --git a/test/geotiff.spec.js b/test/geotiff.spec.js index 327462d8..08cfcabb 100644 --- a/test/geotiff.spec.js +++ b/test/geotiff.spec.js @@ -1290,9 +1290,8 @@ describe('writeTests', () => { }); describe('BlockedSource Test', () => { - const blockedSource = new BlockedSource(null, { blockSize: 2 }); - it('Groups only contiguous blocks as one group', () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); const groups = blockedSource.groupBlocks([2, 0, 1, 3]); expect(groups.length).to.equal(1); const [group] = groups; @@ -1302,6 +1301,7 @@ describe('BlockedSource Test', () => { }); it('Groups two non-contiguous blocks as two groups', () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); const groups = blockedSource.groupBlocks([0, 1, 7, 2, 8, 3]); expect(groups.length).to.equal(2); const [group1, group2] = groups; @@ -1314,4 +1314,84 @@ describe('BlockedSource Test', () => { expect(group2.length).to.equal(4); expect(group2.blockIds).to.deep.equal([7, 8]); }); + + it('Fetches only the minimum number of blocks', async () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); + blockedSource.source = { fileSize: null, fetch: async () => [{ data: new Uint8Array(2).buffer, offset: 0 }] }; + const data = await blockedSource.fetch([{ offset: 0, length: 2 }]); + expect(data[0].byteLength).to.equal(2); + }); + + it('Fetches complete first block', async () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); + const data = new Uint8Array([1, 2, 3, 4]).buffer; + blockedSource.source = { + fileSize: null, + fetch: async () => [ + { data, offset: 0 }, + ] }; + const result = await blockedSource.fetch([{ offset: 0, length: 2 }]); + expect(Array.from(new Uint8Array(result[0]))).to.deep.equal([1, 2]); + }); + + it('Fetches complete last block', async () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); + const data = new Uint8Array([1, 2, 3, 4]).buffer; + blockedSource.source = { + fileSize: null, + fetch: async () => [ + { data, offset: 0 }, + ] }; + const result = await blockedSource.fetch([{ offset: 2, length: 2 }]); + expect(Array.from(new Uint8Array(result[0]))).to.deep.equal([3, 4]); + }); + + it('Fetches partial data from the beginning', async () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); + const data = new Uint8Array([1, 2, 3, 4]).buffer; + blockedSource.source = { + fileSize: null, + fetch: async () => [ + { data, offset: 0 }, + ] }; + const result = await blockedSource.fetch([{ offset: 0, length: 1 }]); + expect(Array.from(new Uint8Array(result[0]))).to.deep.equal([1]); + }); + + it('Fetches partial data from the end', async () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); + const data = new Uint8Array([1, 2, 3, 4]).buffer; + blockedSource.source = { + fileSize: null, + fetch: async () => [ + { data, offset: 0 }, + ] }; + const result = await blockedSource.fetch([{ offset: 3, length: 1 }]); + expect(Array.from(new Uint8Array(result[0]))).to.deep.equal([4]); + }); + + it('Fetches data from between the blocks', async () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); + const data = new Uint8Array([1, 2, 3, 4]).buffer; + blockedSource.source = { + fileSize: null, + fetch: async () => [ + { data, offset: 0 }, + ] }; + const result = await blockedSource.fetch([{ offset: 1, length: 2 }]); + expect(Array.from(new Uint8Array(result[0]))).to.deep.equal([2, 3]); + }); + + it('Fetches multiple slices from between blocks', async () => { + const blockedSource = new BlockedSource(null, { blockSize: 2 }); + const data = new Uint8Array([1, 2, 3, 4, 5, 6]).buffer; + blockedSource.source = { + fileSize: null, + fetch: async () => [ + { data, offset: 0 }, + ] }; + const result = await blockedSource.fetch([{ offset: 1, length: 2 }, { offset: 3, length: 2 }]); + expect(Array.from(new Uint8Array(result[0]))).to.deep.equal([2, 3]); + expect(Array.from(new Uint8Array(result[1]))).to.deep.equal([4, 5]); + }); }); From a77717ab6b8175a0d182a46a232ba555a81304d4 Mon Sep 17 00:00:00 2001 From: Piotr Jurczynski <619650+pjurczynski@users.noreply.github.com> Date: Fri, 3 Oct 2025 11:45:22 +0200 Subject: [PATCH 2/2] test: improve test message to better relay what we test --- test/geotiff.spec.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/geotiff.spec.js b/test/geotiff.spec.js index 08cfcabb..a9e05977 100644 --- a/test/geotiff.spec.js +++ b/test/geotiff.spec.js @@ -1,20 +1,20 @@ /* eslint-disable no-unused-expressions */ -import isNode from 'detect-node'; import { expect } from 'chai'; -import http from 'http'; -import serveStatic from 'serve-static'; +import isNode from 'detect-node'; import finalhandler from 'finalhandler'; +import http from 'http'; import AbortController from 'node-abort-controller'; import { dirname } from 'path'; +import serveStatic from 'serve-static'; import { fileURLToPath } from 'url'; -import { GeoTIFF, fromArrayBuffer, writeArrayBuffer, fromUrls, Pool } from '../dist-module/geotiff.js'; -import { makeFetchSource } from '../dist-module/source/remote.js'; -import { makeFileSource } from '../dist-module/source/file.js'; -import { BlockedSource } from '../dist-module/source/blockedsource.js'; -import { chunk, toArray, toArrayRecursively, range } from '../dist-module/utils.js'; import DataSlice from '../dist-module/dataslice.js'; import DataView64 from '../dist-module/dataview64.js'; +import { fromArrayBuffer, fromUrls, GeoTIFF, Pool, writeArrayBuffer } from '../dist-module/geotiff.js'; +import { BlockedSource } from '../dist-module/source/blockedsource.js'; +import { makeFileSource } from '../dist-module/source/file.js'; +import { makeFetchSource } from '../dist-module/source/remote.js'; +import { chunk, range, toArray, toArrayRecursively } from '../dist-module/utils.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -1315,7 +1315,7 @@ describe('BlockedSource Test', () => { expect(group2.blockIds).to.deep.equal([7, 8]); }); - it('Fetches only the minimum number of blocks', async () => { + it('Fetches all data in a single block', async () => { const blockedSource = new BlockedSource(null, { blockSize: 2 }); blockedSource.source = { fileSize: null, fetch: async () => [{ data: new Uint8Array(2).buffer, offset: 0 }] }; const data = await blockedSource.fetch([{ offset: 0, length: 2 }]);