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.

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

Update ChunkedStream.makeSubStream to actually check if (some) data exists when the length parameter is undefined #10696

Merged
merged 2 commits into from
Apr 13, 2019
Merged

Update ChunkedStream.makeSubStream to actually check if (some) data exists when the length parameter is undefined #10696

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49e8a27
Update `ChunkedStream.makeSubStream` to actually check if (some) data…
Snuffleupagus Mar 29, 2019
c6ddbd5
Add a `progressiveDataLength` fast-path to `ChunkedStream.ensureByte`
Snuffleupagus Mar 29, 2019
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 src/core/chunked_stream.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ class ChunkedStream {
}

ensureByte(pos) {
if (pos < this.progressiveDataLength) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume pos is 0-indexed, so the < check will work. However, why are we using <= then in ensureRange below? I think the end is also a 0-indexed position, so it that wrong below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way that I've understood MissingDataException, and related code, is that begin is inclusive while end is exclusive, which should thus explain things; note the formatting used in

pdf.js/src/core/core_utils.js

Lines 32 to 35 in 47f208d

function MissingDataException(begin, end) {
this.begin = begin;
this.end = end;
this.message = `Missing data [${begin}, ${end})`;
and compare with standard interval notation as outlined in https://en.wikipedia.org/wiki/Interval_(mathematics)#Including_or_excluding_endpoints

To be absolutely sure though, I suppose that you might want @yurydelendik to weigh in here!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yurydelendik Sorry to bother you, but any chance that you have time to comment here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, given that a ensureByte(pos) call should essentially be equal to a ensureRange(pos, pos + 1) call, that would mean a if (pos + 1 <= this.progressiveDataLength) { check in the latter case which is how I arrived at the condition under discussion here.

return;
}

const chunk = Math.floor(pos / this.chunkSize);
if (chunk === this.lastSuccessfulEnsureByteChunk) {
return;
Expand Down