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

[php8-compat] Upgrade TCPDF version to support php8 #20540

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

seamuslee001
Copy link
Contributor

Overview

This upgrades the TCPDF Library to be compatible with php8

Before

TCPDF version not compatible with php8

After

TCPDF compatible with php8

Change log is here https://github.com/tecnickcom/TCPDF/blob/main/CHANGELOG.TXT#L2 and nothing jumps out at me as major issue

ping @demeritcowboy @eileenmcnaughton

@civibot
Copy link

civibot bot commented Jun 7, 2021

(Standard links)

@civibot civibot bot added the master label Jun 7, 2021
@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@demeritcowboy
Copy link
Contributor

I suppose I should test this with Cdntax.

Fix vulnerability to roman numeral bombs

Ah good this was fixed in version VI.IV.0 on MMXXI-III-XXVII

@demeritcowboy
Copy link
Contributor

A couple things threw me with this but in terms of r-run it's fine. Also checked mailing labels.

  1. The version it stamps on pdf files is still 6.3.5 - see https://github.com/tecnickcom/TCPDF/blob/6.4.1/include/tcpdf_static.php#L58
  2. There is no release 6.4.2 yet - does 6.4.1 still meet the php 8 needs?

If this is meeting the php 8 needs then I think it's mergeable.

@seamuslee001
Copy link
Contributor Author

@demeritcowboy I found it worked fine for the unit test issue I had on php8, I'm going to merge based on the above

@seamuslee001 seamuslee001 merged commit 9d11f14 into civicrm:master Jun 8, 2021
@seamuslee001 seamuslee001 deleted the tcpdf_upgrade branch June 8, 2021 22:16
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.

2 participants