LaravelDaily / laravel-invoices

Laravel package to generate PDF invoices from various customizable parameters
GNU General Public License v3.0
1.39k stars 303 forks source link

Added optional 'extension' parameter for the logo type #115

Closed ash-jc-allen closed 3 years ago

ash-jc-allen commented 3 years ago

Hi! First off, huge thanks for building this package. I love how easy it is to work with! 😄

This PR just adds an optional string $extension parameter to the logo() method.

I thought it might be useful because it allows for images to be passed in using signed URLs (such as ones created for S3). For example, in my application, I allow users to upload their own logos that are then placed in the invoices. Because the logos are stored privately, I pass in a temporary signed URL that might look something like this:

https://my-app-name-here.s3.eu-west-2.amazonaws.com/teams/1/logo/HceXWX3oW6we9mqptKomjG3F2HRDA7gYJOoDs0V0.png?X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIA252AD7Q7LRKE5T4G%2F20211005%2Feu-west-2%2Fs3%2Faws4_request&X-Amz-Date=20211005T161702Z&X-Amz-SignedHeaders=host&X-Amz-Expires=60&X-Amz-Signature=372b561db7d4bc749d2c9137b44bcb73a5a7b309f05ebba0396b0cbf2ee171f5

This means that in the getLogo() method, the pathinfo() method detects this as being the file extension:

png?X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIA252AD7Q7LRKE5T4G%2F20211005%2Feu-west-2%2Fs3%2Faws4_request&X-Amz-Date=20211005T161702Z&X-Amz-SignedHeaders=host&X-Amz-Expires=60&X-Amz-Signature=372b561db7d4bc749d2c9137b44bcb73a5a7b309f05ebba0396b0cbf2ee171f5

Because of this, it makes the mime-type in the logo string returned from the getLogo() include the query params from the URL and so it can't display the image.

So, with this new change, you could manually pass the expected file extension when setting the path for the logo like so:

$invoiceFile = Invoice::make('invoice')->logo($signedLogoPathHere, 'png');

Hopefully, this is something that you think would be worthwhile and helpful. If it's something you might consider pulling in but might need changes, let me know and I'll make them ASAP

mc0de commented 3 years ago

hi, this function is supposed to do 1 thing, get local image. by adding more logic behind the scenes adds unnecessary complexity.

basically your solution is a dirty "hack".

it is really not about signed urls, but about remote urls with parameters, so, going with that:

you're fortunate that file_get_contents fetches your remote url, but this is not the case on all environments, it depends on php settings, and this implementation going to fail for others.

I'd suggest in your project to go with different workflow and keep ->logo() function as it was intended to be, we do not need to bundle everything that is easily done on demand on project basis.

$url  = 'http://www.example.com/my-image.png';
$path = '/path/to/my-image.png';

$ch = curl_init($url);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
$data = curl_exec($ch);
curl_close($ch);
file_put_contents($path, $data);

$invoice->logo($path);