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

JpegEmbedder: Support Buffers that have data in their backing buffer is offset #1273

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

allenbenz-doma
Copy link

@allenbenz-doma allenbenz-doma commented Jul 12, 2022

What?

This adds support when the jpeg image data in the backing buffer for a Buffer is offset from index 0.

Why?

Buffer.buffer isn't necessarily indexed the same as the Buffer.

Ran into this when some transparent optimization caused a single buffer to be used for multiple short hardcoded test image/non-image Buffers.
https://stackoverflow.com/a/50118846 talks about it, the gist of it that Buffer.from can call Buffer.allocUnsafe which may reuse memory from a pool, so image data can be stored in the middle of the buffer.

Someone could deliberately do this too with something like a sprite sheet.

How?

DataView already supports handling this and Uint8Array has the necessary offset/length fields, so added them to the DataView constructor.

Testing?

Wrote a new test, failed before change, passed after, existing tests pass, scope of the change is very small.
Only ran the nodejs and browser integration tests.

New Dependencies?

No

Screenshots

N/A

Suggested Reading?

https://nodejs.org/dist/latest-v10.x/docs/api/buffer.html#buffer_class_method_buffer_allocunsafe_size

Anything Else?

Checklist

  • I read CONTRIBUTING.md.
  • I read MAINTAINERSHIP.md#pull-requests.
  • I added/updated unit tests for my changes.
  • I added/updated integration tests for my changes.
  • I ran the integration tests.
  • I tested my changes in Node, Deno, and the browser.
  • I viewed documents produced with my changes in Adobe Acrobat, Foxit Reader, Firefox, and Chrome.
  • I added/updated doc comments for any new/modified public APIs.
  • My changes work for both new and existing PDF files.
  • I ran the linter on my changes.

@allenbenz-doma allenbenz-doma changed the title Support Buffers that have data in their backing buffer is offset JpegEmbedder: Support Buffers that have data in their backing buffer is offset Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant