[12.x] Fix image support in PDF invoices #1045
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@thijskok shouldn't |
That's an interesting question. I didn't change anything about the 1.0 constraint. It might be safer, but then again, they did introduce a "breaking" change in a patch version (0.8.5 => 0.8.6)... I'd bump it to be on the safe side. @taylorotwell did merge it already though (with my eternal thanks 👍 ).
|
|
Yeah seems this security fix was also patched for 1.x in 1.0.1 so I'll bump it. Will tag these changes on Tuesday. |
Awesome! Thanks @driesvints . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes (local) image support when using the invoice PDF feature.
Background
With the release of Dompdf 0.8.6, a disclosure vulnerability was fixed. By doing so, a breaking change was introduced: when generating a PDF without the proper configuration, only resources within the dompdf folder itself are accessible. As a result, when using Cashier to generate a PDF invoice, the use of a local image (for example, a logo) is blocked.
Example
Consider this example:
Before 0.8.6, this worked fine. As of 0.8.6, only resources in the vendor/dompdf directory are allowed, causing any file in
public_path(or any other path in your Laravel app, for that matter) to be rejected.Solution
The solution is to configure Dompdf using the new chroot setting. This PR sets the chroot to
base_path. As a side effect, the version constraint for dompdf has to be set to 0.8.6 - earlier versions lack this setting.There's no test available for handling PDF's in general, so I refrained from writing one. If this is required, I am willing to take a stab at it.
Workarounds