Skip to content

Stop range-request truncation breaking a second kpub load #15105

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.

Sign up for GitHub

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

Open
rtibblesbot wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Stop range-request truncation breaking a second kpub load #15105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
67fecb6
Stop AdaptiveHttpReader issuing overlapping range requests
rtibblesbot Jul 30, 2026
63da9d6
Defer a range request that overlaps one already in flight
rtibblesbot Jul 31, 2026
24663be
Refetch a truncated range response past the browser cache
rtibblesbot Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/kolibri-zip/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ Chunks: [=====chunk1=====] [===chunk2===]

This reduces the number of HTTP requests when extracting multiple files. Concurrent reads from the same chunk are deduplicated into a single fetch.

Range requests for one URL never overlap: chunks end at the following entry's offset, a read spilling past a chunk is served from both chunks, and one reaching the prefetched tail grows the tail downwards. A request that would still overlap one in flight waits for it rather than issuing.

That does not make a short response impossible: Chromium's in-memory cache serves a partial hit as fewer bytes than the `Content-Length` it declares, sized to a 4096-byte sparse-entry boundary (#15103). The body is always a correct prefix, so a short one is refetched with `Cache-Control: no-cache`, and only becomes an error — a described one, rather than an opaque inflater `TypeError` — if that is short too.

### Large Media File Handling

For large video/audio files, provide a `largeFileUrlGenerator` to serve them directly:
Expand Down
221 changes: 177 additions & 44 deletions packages/kolibri-zip/src/AdaptiveHttpReader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ const DEFAULT_CHUNK_SIZE = 500 * 1024; // 500KB
const ZIP_LOCAL_HEADER_SIZE = 30;
// Conservative estimate for extra field length (varies, typically 0-100 bytes).
// ZIP64 extensions alone add 28 bytes, and tools may write timestamps, Unicode paths, etc.
// Overestimating wastes negligible bandwidth; underestimating risks missing chunk coverage.
// Overestimating wastes negligible bandwidth; underestimating risks missing chunk
// coverage. Only the last entry relies on it; the rest are clamped to the next offset.
const ZIP_EXTRA_FIELD_ESTIMATE = 100;

// Tail prefetch heuristics for Central Directory estimation
Expand All @@ -25,6 +26,11 @@ const CD_SIZE_RATIO = 0.03; // Estimate CD is ~3% of total file size
// zip.js reads near the end of file looking for EOCD signature.
const EOCD_PROXIMITY_THRESHOLD = 100;

// Range requests in flight, keyed by URL, so an overlapping pair is never issued together
// (#15103). Shared across readers: one built on navigation can outlive the requests of the
// one it replaced.
const inFlightRanges = new Map();

/**
* AdaptiveHttpReader extends zip.js's Reader to provide adaptive fast/lazy loading for zip files.
*
Expand Down Expand Up @@ -70,6 +76,7 @@ export default class AdaptiveHttpReader extends Reader {
this._fullData = null;
this._chunks = null;
this._tailChunk = null;
this._tailPromise = null;
this.size = 0;
}

Expand Down Expand Up @@ -199,39 +206,56 @@ export default class AdaptiveHttpReader extends Reader {
return this._readFromChunk(chunk, index, length);
}

// Prefetch tail on first read near end of file (EOCD triggers this)
if (!this._tailChunk && index + length >= this.size - EOCD_PROXIMITY_THRESHOLD) {
// Use promise deduplication to prevent concurrent tail fetches
if (!this._tailFetching) {
const tailSize = this._estimateTailSize();
const tailStart = Math.max(0, this.size - tailSize);
this._tailFetching = this._rangeRequest(tailStart, this.size - tailStart).then(tailData => {
this._tailChunk = { startOffset: tailStart, endOffset: this.size, data: tailData };
this._tailFetching = null;
return this._tailChunk;
});
}
const tailChunk = await this._tailFetching;
// The tail chunk always extends to EOF (endOffset === this.size), so reads
// can only miss if they start before the cached region. This happens when
// our CD size estimate was too small and zip.js needs earlier bytes.
if (index < tailChunk.startOffset) {
return this._rangeRequest(index, length);
}
const relativeStart = index - tailChunk.startOffset;
return tailChunk.data.slice(relativeStart, relativeStart + length);
}

// Before configureChunks() is called, allow range requests for zip parsing
if (!this.chunksConfigured) {
return this._rangeRequest(index, length);
// Before a tail exists, EOF proximity seeds one; after, any read reaching it grows it down.
const tailReach = this._tailChunk?.startOffset ?? this.size - EOCD_PROXIMITY_THRESHOLD;
if (index + length >= tailReach) {
await this._ensureTail(index);
}
return this._readRegion(index, index + length);
}

// After chunks are configured, all reads should be covered by chunks
throw new Error(
`No chunk covers range ${index}-${index + length}. ` +
`This may indicate a large file being extracted without a largeFileUrlGenerator.`,
return this._readAcrossChunks(index, length);
}

/**
* Read a span no single chunk covers from the chunks it crosses. Chunk ends are clamped
* to the next entry's offset, so zip.js's fixed 16-byte data-descriptor read can cross a
* boundary; requesting the span itself would overlap those chunks.
* @param {number} index - Start offset
* @param {number} length - Number of bytes to read
* @returns {Promise<Uint8Array>} The requested data
*/
async _readAcrossChunks(index, length) {
const end = index + length;
// Walk the span first, so an uncovered span throws before anything is fetched.
const spanning = [];
let offset = index;
while (offset < end) {
const chunk = this._findChunk(offset);
if (!chunk) {
throw new Error(
`No chunk covers range ${index}-${end}. ` +
`This may indicate a large file being extracted without a largeFileUrlGenerator.`,
);
}
spanning.push(chunk);
offset = chunk.endOffset;
}

const result = new Uint8Array(length);
await Promise.all(
spanning.map(chunk => {
const partStart = Math.max(chunk.startOffset, index);
const partLength = Math.min(chunk.endOffset, end) - partStart;
return this._readFromChunk(chunk, partStart, partLength).then(part =>
result.set(part, partStart - index),
);
}),
);
return result;
}

/**
Expand Down Expand Up @@ -261,17 +285,74 @@ export default class AdaptiveHttpReader extends Reader {
}

/**
* Make a range request for specific bytes.
* Make a range request for specific bytes, held back until no request for this URL has
* an overlapping range in flight.
* @param {number} start - Start offset
* @param {number} length - Number of bytes to read
* @returns {Promise<Uint8Array>} The requested data
*/
_rangeRequest(start, length) {
async _rangeRequest(start, length) {
const end = start + length;
const inFlight = inFlightRanges.get(this.url) ?? new Set();
inFlightRanges.set(this.url, inFlight);

const overlapping = () => [...inFlight].filter(r => r.start < end && start < r.end);
// Re-checked after each wait: another deferred request may have started meanwhile.
for (let waiting = overlapping(); waiting.length; waiting = overlapping()) {
await Promise.allSettled(waiting.map(r => r.response));
}

const request = { start, end, response: this._fetchCompleteRange(start, length) };
inFlight.add(request);
try {
return await request.response;
} finally {
inFlight.delete(request);
if (!inFlight.size) {
inFlightRanges.delete(this.url);
}
}
}

/**
* Fetch a range, re-fetching past the browser cache if the response is short.
* @param {number} start - Start offset
* @param {number} length - Number of bytes to read
* @returns {Promise<Uint8Array>} The requested data
*/
async _fetchCompleteRange(start, length) {
const body = await this._sendRangeXhr(start, length);
if (body.byteLength >= length) {
return body;
}
// No caller asks past EOF, so a short body is a partial cache hit (#15103).
const refetched = await this._sendRangeXhr(start, length, true);
if (refetched.byteLength < length) {
throw new Error(
`Truncated range response for bytes=${start}-${start + length - 1}: ` +
`expected ${length} bytes, received ${body.byteLength}, ` +
`then ${refetched.byteLength} with the cache bypassed`,
);
}
return refetched;
}

/**
* Issue the range request itself, resolving whatever body arrives.
* @param {number} start - Start offset
* @param {number} length - Number of bytes to read
* @param {boolean} [bypassCache] - Force a network fetch rather than a cached response
* @returns {Promise<Uint8Array>} The response body, of any length
*/
_sendRangeXhr(start, length, bypassCache = false) {
return new Promise((resolve, reject) => {
const xhr = new XMLHttpRequest();
xhr.open('GET', this.url);
xhr.responseType = 'arraybuffer';
xhr.setRequestHeader('Range', `bytes=${start}-${start + length - 1}`);
if (bypassCache) {
xhr.setRequestHeader('Cache-Control', 'no-cache');
}

xhr.addEventListener('load', () => {
if (xhr.status === 206 || xhr.status === 200) {
Expand All @@ -286,6 +367,54 @@ export default class AdaptiveHttpReader extends Reader {
});
}

/**
* Read [start, end), requesting only the bytes the prefetched tail does not already
* hold so the request cannot overlap the tail's own.
* @param {number} start - Start offset
* @param {number} end - End offset, exclusive
* @returns {Promise<Uint8Array>} The requested bytes
*/
async _readRegion(start, end) {
// Clamp to EOF: the last chunk's end is an estimate, and a short body must stay a fault.
const stop = Math.min(end, this.size);
const tail = this._tailChunk;
if (!tail || stop <= tail.startOffset) {
return this._rangeRequest(start, stop - start);
}
if (start >= tail.startOffset) {
return tail.data.slice(start - tail.startOffset, stop - tail.startOffset);
}
const head = await this._rangeRequest(start, tail.startOffset - start);
const cached = tail.data.subarray(0, stop - tail.startOffset);
const region = new Uint8Array(head.length + cached.length);
region.set(head);
region.set(cached, head.length);
return region;
}

/**
* Seed or grow the tail chunk downwards to cover minStart, so a read below an
* underestimated central directory never re-requests the tail's bytes.
* @param {number} minStart - Lowest offset that must be covered
* @returns {Promise} Resolves once _tailChunk covers [<= minStart, size)
*/
_ensureTail(minStart) {
const startOffset = Math.min(minStart, Math.max(0, this.size - this._estimateTailSize()));
// Chained, so concurrent callers each fetch only the bytes the tail still lacks.
this._tailPromise = Promise.resolve(this._tailPromise).then(async () => {
if (this._tailChunk && startOffset >= this._tailChunk.startOffset) {
return;
}
// _readRegion splices in whatever the current tail already holds.
this._tailChunk = {
startOffset,
endOffset: this.size,
data: await this._readRegion(startOffset, this.size),
};
});
return this._tailPromise;
}

/**
* Build chunk boundaries from entry metadata.
* Groups adjacent small files into chunks of approximately chunkSize.
Expand All @@ -294,17 +423,19 @@ export default class AdaptiveHttpReader extends Reader {
* @returns {Array} Array of chunk objects with startOffset, endOffset, data, fetching
*/
_buildChunks(entries) {
// Filter out directories and entries without offset, sort by offset
const fileEntries = entries
.filter(e => !e.directory && e.offset !== undefined)
.sort((a, b) => a.offset - b.offset);

if (fileEntries.length === 0) return [];
const sorted = entries.filter(e => e.offset !== undefined).sort((a, b) => a.offset - b.offset);

const chunks = [];
let currentChunk = null;

for (const entry of fileEntries) {
for (let i = 0; i < sorted.length; i++) {
const entry = sorted[i];
// Skipped rather than filtered out: a directory holds no data, but its local header
// still bounds the entry before it.
if (entry.directory) {
continue;
}

// Only exclude files that will be served via URL generation (large audio/video).
// Large non-media files (e.g. JS libraries, images) must remain in chunks
// so they can be extracted in the frontend.
Expand All @@ -319,12 +450,15 @@ export default class AdaptiveHttpReader extends Reader {
}

const entryStart = entry.offset;
const entryEnd =
// The next entry's offset is a hard upper bound on where this entry's data can end.
const entryEnd = Math.min(
entry.offset +
ZIP_LOCAL_HEADER_SIZE +
entry.filename.length +
ZIP_EXTRA_FIELD_ESTIMATE +
entry.compressedSize;
ZIP_LOCAL_HEADER_SIZE +
entry.filename.length +
ZIP_EXTRA_FIELD_ESTIMATE +
entry.compressedSize,
sorted[i + 1]?.offset ?? Infinity,
);

if (!currentChunk) {
// Start a new chunk with this entry
Expand Down Expand Up @@ -403,8 +537,7 @@ export default class AdaptiveHttpReader extends Reader {
// Ensure chunk data is loaded (with promise deduplication for concurrent reads)
if (!chunk.data) {
if (!chunk.fetching) {
const chunkLength = chunk.endOffset - chunk.startOffset;
chunk.fetching = this._rangeRequest(chunk.startOffset, chunkLength).then(data => {
chunk.fetching = this._readRegion(chunk.startOffset, chunk.endOffset).then(data => {
chunk.data = data;
chunk.fetching = null;
});
Expand Down
Loading
Loading