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

I have a DivisionByZeroError due to a null value in the division #3530

Open
mmanzano opened this issue Sep 24, 2024 · 5 comments · May be fixed by #3571
Open

I have a DivisionByZeroError due to a null value in the division #3530

mmanzano opened this issue Sep 24, 2024 · 5 comments · May be fixed by #3571
Milestone

Comments

@mmanzano
Copy link

This line is protecting us from 0.0 value but not for null values. Is there a reason to not think about an scenario where $rightValue could be null?

if ($rightValue === 0.0) {

I have problems due to that when I use tailwind in order to styling a document. In my end when I remove the line @tailwind components the issue is gone so I don't know if something is wrong in Tailwind. I've tried to change the condition to $rightValue === null || $rightValue === 0.0 and it works. Could I make a PR with this?. I'm asking because maybe I'm missing something. I don't want to do the PR without explaining my reasoning before.

@bsweeney
Copy link
Member

Pull requests are always welcome. It would help diagnose the issue to know what from the CSS is causing the issue, but if you are unable to identify it I might be able to do so if you provide some sample code.

@mmanzano
Copy link
Author

I leave here a repo where you can reproduce the error:

https://github.com/mmanzano/dompdf-issue-3530

The problematic CSS is in this file:

https://github.com/mmanzano/dompdf-issue-3530/blob/main/resources/views/pdf.blade.php

If I'm not wrong, It's something related with the calc in the padding-bottom of the .aspect-w-x css classes.

I left in the readme some instructions in order to reproduce the exception in a computer without php installed (just in case). Let me know if you need more directions:

https://github.com/mmanzano/dompdf-issue-3530/blob/main/README.md

Thanks for your answer. 👍

@mmanzano
Copy link
Author

mmanzano commented Jan 8, 2025

@bsweeney did you have time to take a look into the issue? ❤️

@bsweeney bsweeney added this to the 3.1.0 milestone Jan 9, 2025
@bsweeney
Copy link
Member

bsweeney commented Jan 9, 2025

I have not had a chance to look into it yet. I can confirm this is happening. Looks like the calc function is running even though the variable hasn't been set. Additionally, the calc isn't re-run later once the variable is set. I'll see if I can figure out how to address (besides adding a null check).

@bsweeney
Copy link
Member

bsweeney commented Jan 11, 2025

So, there are two issues here:

  1. Attempting to compute a calculation where one side is null. I think if either side of the calculation is null we should not perform the calculation.
  2. Unsupported selectors. Dompdf doesn't yet support selectors with escaped characters. The problematic variable is set by the selector .aspect-w-1\.41. The "." is supposed to represent a literal period in the selector, but Dompdf doesn't currently understand escaped characters. As a result, this selector actually is taken to represent as an element with two classes applied: "aspect-w-1" and "41".

I should be able to address both issues in the next release. In the meantime, I believe if you just replace the escaped characters (e.g., change the class to something like aspect-w-1_41) then your document should render without errors.

bsweeney added a commit that referenced this issue Jan 11, 2025
This change prevents logic or rendering errors in scenarios where a CSS calc function utilizes a non-existent variable as one of the operands.

ref #3530
bsweeney added a commit that referenced this issue Jan 11, 2025
@bsweeney bsweeney linked a pull request Jan 11, 2025 that will close this issue
bsweeney added a commit that referenced this issue Jan 11, 2025
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 a pull request may close this issue.

2 participants