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

Ensure header/footer temp files are closed before wkhtmltopdf uses them #1054

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

Conversation

camilova
Copy link

@camilova camilova commented Apr 5, 2023

Recently was applied the PR #1039 but it was failing on windows environment causing permission denied error on created headers and footers temp files. This was caused in my case because the new way to handle temp files does not .close the files, causing an access error on windows env when wkhtmltopdf attempts to use them later. This PR fix the problem closing the files after creation and let them free to be used by another process on windows (and also linux).

On the other hand, the new variable used to hold the temp files objects prevents that files were destroyed by garbage collector (the original issue), but when one reuse the same WickedPdf instance, the variable hf_tempfiles on OptionParser class is not cleared resulting on failiures when it tries to delete files that dont exists anymore (because there was information created on a previous use of the WickedPdf class). This fix that situation moving the logic of cleaning tempfiles at OptionParser class, and clearing the hf_tempfiles after remove the files, letting it clean for a new call of its methods.

And the last change that I made is using URI to create local files url (File:///) to follow the convention of ruby and make it more secure and standarized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant