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

Make FpdfOutput work with allow_url_fopen=0 #192

Closed
kasoft opened this issue Sep 9, 2022 · 10 comments
Closed

Make FpdfOutput work with allow_url_fopen=0 #192

kasoft opened this issue Sep 9, 2022 · 10 comments
Labels
bug Something isn't working support Support & help third-party-bug A bug from a different provider is blocking

Comments

@kasoft
Copy link

kasoft commented Sep 9, 2022

For security reasons, my Php Settings do not allow to directly pass the Qr Code from Qr Bill to FPDF as a base64 encoded String (allow_url_fopen=0). As the Class FpdF is final, there is no way to solve this by extending the Class. I would suggest to implement an option where you can define a path or tempfile. If this is set, the Qr Code could be saved there. I can try to figure something out. What so you think?

private function addSwissQrCodeImage(): void
    {
        $qrCode = $this->getQrCode();

        $yPosQrCode = 209.5 + $this->offsetY;
        $xPosQrCode = 67 + $this->offsetX;
        
        if(!empty($this->qrCodeFile)) {
            $qrCode->writeFile($this->qrCodeFile);
            $this->fpdf->Image($this->qrCodeFile, $xPosQrCode, $yPosQrCode, 46, 46, 'png');
            unlink($this->qrCodeFile);
        } else {
            $this->fpdf->Image($qrCode->getDataUri($this->getQrCodeImageFormat()), $xPosQrCode, $yPosQrCode, 46, 46, 'png');
        }
        
    }

@sprain
Copy link
Owner

sprain commented Sep 9, 2022

I could reproduce the problem and got this message:

fopen(): data:// wrapper is disabled in the server configuration by allow_url_fopen=0 in /var/www/html/vendor/fpdf/fpdf/src/Fpdf/Fpdf.php on line 1259
Fatal error: Uncaught Exception: FPDF error: Can't open image file: data:image/png;base64,iVBORw0KGgoAAAANSUhEUg …

I agree it's good practice to have allow_url_fopen disabled.

Not sure yet how to deal with it. How do other projects using FPDF do it?

@sprain sprain added bug Something isn't working third-party-bug A bug from a different provider is blocking support Support & help help wanted Extra attention is needed labels Sep 9, 2022
@kasoft
Copy link
Author

kasoft commented Sep 9, 2022

Maybe it's not that comfortable, but the file could be temoprarily saved (to a tmp folder), passed to fpdf to generate the Pdf File and deleteded afterwards. This is the solution i realized (see example above).

@kohlerdominik
Copy link
Contributor

@kasoft

Can you check wether script45 Memory Image Support works with allow_url_fopen disabled?

@kasoft
Copy link
Author

kasoft commented Sep 9, 2022

@kasoft

Can you check wether script45 Memory Image Support works with allow_url_fopen disabled?

Yes, i’ll check on Monday

@kasoft
Copy link
Author

kasoft commented Sep 13, 2022

With the proposed script it will work.

$qr = $qrCode->getAsString($this->getQrCodeImageFormat());
$this->fpdf->MemImage($qr, 50, 30);

@kohlerdominik
Copy link
Contributor

kohlerdominik commented Sep 13, 2022

Hi @kasoft

Can you test this code please:

https://github.com/sprain/php-swiss-qr-bill/blob/master/src/PaymentPart/Output/FpdfOutput/FpdfOutput.php#L95

    private function addSwissQrCodeImage(): void
    {
        $qrCode = $this->getQrCode();

        $yPosQrCode = 209.5 + $this->offsetY;
        $xPosQrCode = 67 + $this->offsetX;

        // vvv   start of changes   vvv
        if (ini_get('allow_url_fopen') === "1") {
            $this->fpdf->Image($qrCode->getDataUri($this->getQrCodeImageFormat()), $xPosQrCode, $yPosQrCode, 46, 46, 'png');
        }
        elseif (method_exists($this->fpdf, 'MemImage')) {
            $qr = $qrCode->getAsString($this->getQrCodeImageFormat());
            $this->fpdf->MemImage($qr, $xPosQrCode, $yPosQrCode, 46, 46);
        } else {
            throw new \RuntimeException('If directive "allow_url_fopen" is disabled, FPDF "Memory Image Support" is required');
        }
        /// ^^^   end of changes   ^^^
    }

@sprain what do you think? I always prefer implicit before explicit. So the two conditons added here may hit the performance a little, but at least it's configured implicitely rather than having to configure some temp paths by the user.

@sprain
Copy link
Owner

sprain commented Sep 14, 2022

Thanks for all the work here!

The code looks fine to me.

I just wonder whether Memory Image Support is something that is generally known to FDPF users or whether we just generate more support requests that way.

Also: we'll need to check whether TcPDF might have the same issue.

@kohlerdominik
Copy link
Contributor

FPDF misses a major feature by not supporting images from memory or as strings natively. But there's nothing we can do about that.

Using temporary files seems terrible for me. Because it also comes with the necessity to manage files (writable directories, unique file names, delete image after generation, generally bad performance, ...). I predict, that this is hard to maintain during evolution of our code, and can also create big issues for users when the temporary files are not managed properly. So not necessary less support cases.

IMO, it's the best to include a DomPDF-output to this library, which is a reasonable dependency for users that require to make PDFs right in PHP. So for less expirienced users, the support case would be closed by recommending to switch to DomPDF, which is a simple composer require and copy-paste from the example then. For expirienced users, adding MemImage to FPDF should be doable, but outside of our support scope.

See #178 and DomPDF on Packagist and Github.

@kasoft
Copy link
Author

kasoft commented Sep 15, 2022

Hi @kohlerdominik

Thanks for the solution. I can confirm that it worked. Only the Line about the Position and the Size of the QR Code had to be adjusted.

$this->fpdf->MemImage($qr, $xPosQrCode, $yPosQrCode, 46, 46);

@sprain sprain removed the help wanted Extra attention is needed label Nov 20, 2022
@sprain
Copy link
Owner

sprain commented Nov 20, 2022

FYI: This is now solved in v4.7.

@sprain sprain closed this as completed Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working support Support & help third-party-bug A bug from a different provider is blocking
Projects
None yet
Development

No branches or pull requests

3 participants