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

fix: base64 images not rendered inside of html #671

Closed
wants to merge 2 commits into from

Conversation

compojoom
Copy link

I'm not sure when this happened, but default base64 encoded images were no longer rendered in html text.

If the image is rendered like this:

<img src="data:image/png;base64,....." />

the image was not rendered when inside of an html text. Instead one has to replace data:image/png;base64, with @which in my opinion is not ideal and an overhead.

#179

@CLAassistant
Copy link

CLAassistant commented Dec 1, 2023

CLA assistant check
All committers have signed the CLA.

include/tcpdf_static.php Outdated Show resolved Hide resolved
@nicolaasuni
Copy link
Member

Please try to resolve the conflicts.

@compojoom compojoom force-pushed the patch-1 branch 4 times, most recently from a31832a to 605d0a1 Compare April 22, 2024 05:42
@compojoom
Copy link
Author

@williamdes I rebased. Sorry for the changelog change, but it seems you had an empty space and my IDE wants to remove it :)

@williamdes
Copy link
Contributor

williamdes commented Apr 22, 2024

@williamdes I rebased. Sorry for the changelog change, but it seems you had an empty space and my IDE wants to remove it :)

That's okay

But can you remove the other commits?
git reset hard on the base and cherry pick 605d0a1 then force push?

compojoom and others added 2 commits April 22, 2024 08:02
I'm not sure when this happened, but default base64 encoded images were no longer rendered in html  text.

If the image is rendered like this:
```
<img src="data:image/png;base64,....." />
```

the image was not rendered when inside of an html text. Instead one has to replace `data:image/png;base64,` with `@`which in my opinion is not ideal and an overhead.

tecnickcom#179

Update include/tcpdf_static.php

Co-authored-by: William Desportes <williamdes@wdes.fr>
@compojoom
Copy link
Author

how about now?

Copy link
Contributor

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect

@nicolaasuni
Copy link
Member

Unfortunately the proposed change has security implications as the file_exist function is used in other places too.
This may allow an attacker to provide arbitrary executable content somewhere.
A proper solution will require a bit of thinking but I'd rather put the energy to complete tc-lib-pdf that already has 90+% of the TCPDF features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants