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

Refactor so images via Pathname are read in binmode #587

Merged
merged 1 commit into from Nov 24, 2013

Conversation

Projects
None yet
2 participants
@fidothe
Copy link
Contributor

fidothe commented Nov 24, 2013

  • Separate out image IO opening and verification logic
  • test that embedded images parse correctly (this was the problem that
    was being masked before)
  • Add simple matcher to tidy up checking XObject parseability
  • fixes #570:
  • don't rely on binmode being defined on IO-like objects (re: #585)

Pathname instances respond to #read but not #binmode, and the old code
was assuming that anything which responded to #read was an IO, with the
result that Pathname instances were beign treated as IO's and getting
read called on them, bypassing File.binread (things responding
to #binmode have that called so with a real IO it would have #read
called after it had been put into binmode...

The fix is simple enough: treat everything that responds to rewind as
an IO (IO's and Files), and anything else treat as a path, and open it
'rb'.

The call to #binmode is guarded because of #585, and the chances of other
IO-alikes not implementing binmode either.

Refactor so images via Pathname are read in binmode
* Separate out image IO opening and verification logic
* test that embedded images parse correctly (this was the problem that
  was being masked before)
* Add simple matcher to tidy up checking XObject parseability
* fixes #570:
* don't rely on binmode being defined on IO-like objects (re: #585)

Pathname instances respond to #read but not #binmode, and the old code
was assuming that anything which responded to #read was an IO, with the
result that Pathname instances were beign treated as IO's and getting
read called on them, bypassing File.binread (things responding
to #binmode have that called so with a real IO it would have #read
called after it had been put into binmode...

The fix is simple enough: treat everything that responds to `rewind` as
an IO (IO's and Files), and anything else treat as a path, and open it
'rb'.
@fidothe

This comment has been minimized.

Copy link
Contributor Author

fidothe commented Nov 24, 2013

I'm going to go ahead and merge this back in - the problem the original caused in #585 is well understood (it was a bug in Paperclip, who were supplying an object which wrapped an IO, and delegated a lot of the IO methods, but didn't delegate binmode) and this tweaked version duck-types IO's using rewind, with a guarded call to binmode.

fidothe added a commit that referenced this pull request Nov 24, 2013

Merge pull request #587 from prawnpdf/image_binmode_bugfix_take2
Refactor so images via Pathname are read in binmode

@fidothe fidothe merged commit ec08e0e into master Nov 24, 2013

1 check passed

default The Travis CI build passed
Details
@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Nov 24, 2013

Thanks!

@practicingruby practicingruby deleted the image_binmode_bugfix_take2 branch Jan 2, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.