spatie / laravel-pdf

Create PDF files in Laravel apps
https://spatie.be/docs/laravel-pdf
MIT License
681 stars 53 forks source link

[Bug]: @inlinedImage only works with static urls #84

Closed okaufmann closed 7 months ago

okaufmann commented 7 months ago

What happened?

The new Blade directive @inlinedImage only works when passing static strings as image URLs or paths. When dynamically passing value from a Variable it does not work, as it does not resolve the value correctly.

This does work:

<div>
   @inlinedImage('https://picsum.photos/200/300')
</div>

This does NOT work:

<div>
   @php $src = 'https://picsum.photos/200/300' @endphp
   @inlinedImage($src)
</div>

This is an example of an Error I've got:

file_get_contents(public/$src): Failed to open stream: No such file or directory

Should a Blade directive not just return valid PHP source code to be evaluated by the view engine to work properly?

IMHO this should be more like this:

Blade::directive('inlinedImage', function ($expression) {
            return "<?php
                \$url = \Illuminate\Support\Str::of(with({$expression}))->trim(\"'\")->trim('\"')->__toString();

                if (! \Illuminate\Support\Str::of(\$url)->startsWith(['http://', 'https://'])) {
                    \$path = public_path(\$url);
                    if (file_exists(\$path)) {
                        \$imageContent = 'data:image/png;base64,' . base64_encode(file_get_contents(\$path));
                        echo '<img src=\"' . \$imageContent . '\" style=\"display:inline;\">';
                    }
                } else {
                    try {
                        \$response = \Illuminate\Support\Facades\Http::get(\$url);
                        if (\$response->successful()) {
                            \$imageContent = 'data:image/png;base64,' . base64_encode(\$response->body());
                            echo '<img src=\"' . \$imageContent . '\" style=\"display:inline;\">';
                        }
                    } catch (\Exception \$e) {
                        throw new \RuntimeException('Failed to fetch the image: ' . \$e->getMessage());
                    }
                }
            ?>";
        });

If someone can confirm this error does occur too, I will create a PR for fixing this.

How to reproduce the bug

Use the inlinedImage Blade directive provided by this package.

<div>
   @php $src = 'https://picsum.photos/200/300' @endphp
   @inlinedImage($src)
</div>

Package Version

1.2.0

PHP Version

8.2.0

Laravel Version

10.43.0

Which operating systems does with happen with?

macOS

Notes

No response

RVP04 commented 7 months ago

I am using both it works fine for me

ArielMejiaDev commented 7 months ago

@okaufmann @RVP04 the issue exists, if you are not able to see it try to clear views php artisan view:clear and you would be able to see it.

It works for static string values, but it fails with variables, property variables or array keys, to fix this I add a PR #85

Here is a quick explanation about it:

https://www.loom.com/share/00cc167045d04310ae194ac2af157d07?sid=a984958f-c830-4077-b263-684eda2378d6

RVP04 commented 7 months ago

Will check and update you . Because I am not passing as a variable

okaufmann commented 7 months ago

@ArielMejiaDev Thank you for explaining this. The only thing that is still not working is, that the error handling is not happening at the right place. The error you forced by setting a non-existing URL was not a RuntimeException.

This is what I do have right now:

Blade::directive('inlinedImage', function ($url) {
            return "<?php
                \$url = \Illuminate\Support\Str::of(with({$url}))->trim(\"'\")->trim('\"')->__toString();

                if (! \Illuminate\Support\Str::of(\$url)->startsWith(['http://', 'https://'])) {
                    \$path = public_path(\$url);
                    if (file_exists(\$path)) {
                        \$imageContent = 'data:image/png;base64,' . base64_encode(file_get_contents(\$path));
                        echo '<img src=\"' . \$imageContent . '\" style=\"display:inline;\">';
                    }
                } else {
                    try {
                        \$response = \Illuminate\Support\Facades\Http::get(\$url);
                        if (\$response->successful()) {
                            \$imageContent = 'data:image/png;base64,' . base64_encode(\$response->body());
                            echo '<img src=\"' . \$imageContent . '\" style=\"display:inline;\">';
                        }
                    } catch (\Exception \$e) {
                        throw new \RuntimeException('Failed to fetch the image: ' . \$e->getMessage());
                    }
                }
            ?>";
        });

Can you confirm?

ArielMejiaDev commented 7 months ago

@ArielMejiaDev Thank you for explaining this. The only thing that is still not working is, that the error handling is not happening at the right place. The error you forced by setting a non-existing URL was not a RuntimeException.

This is what I do have right now:

Blade::directive('inlinedImage', function ($url) {
            return "<?php
                \$url = \Illuminate\Support\Str::of(with({$url}))->trim(\"'\")->trim('\"')->__toString();

                if (! \Illuminate\Support\Str::of(\$url)->startsWith(['http://', 'https://'])) {
                    \$path = public_path(\$url);
                    if (file_exists(\$path)) {
                        \$imageContent = 'data:image/png;base64,' . base64_encode(file_get_contents(\$path));
                        echo '<img src=\"' . \$imageContent . '\" style=\"display:inline;\">';
                    }
                } else {
                    try {
                        \$response = \Illuminate\Support\Facades\Http::get(\$url);
                        if (\$response->successful()) {
                            \$imageContent = 'data:image/png;base64,' . base64_encode(\$response->body());
                            echo '<img src=\"' . \$imageContent . '\" style=\"display:inline;\">';
                        }
                    } catch (\Exception \$e) {
                        throw new \RuntimeException('Failed to fetch the image: ' . \$e->getMessage());
                    }
                }
            ?>";
        });

Can you confirm?

Thanks @okaufmann let me check, thanks

ArielMejiaDev commented 7 months ago

Thanks @okaufmann for your feedback, based on this discussion I think we got a solutions that resolves the issue and always returns an exception when it fails:

The Last commits make @inlinedImage:

[x] Works with static string as params [x] Works with variables as params [x] Cleanup Blade Directive Code [x] Always returns an exception if an image fails to fetch (absolute path) or is not found (relative path)

Demo

https://www.loom.com/share/a7a760129c93452cb1006ff4cf21de53?sid=73be0c89-de9b-4367-aa6a-8b4c5118bdc1

It should close #84

freekmurze commented 7 months ago

We'll handle this further in #85