Allow phpoffice/phpspreadsheet 2.0+ also

[mwikberg-virta]

1️⃣ Why should it be added? What are the benefits of this change? Newer versions of phpoffice/phpspreadsheet (2.x) exist and someone might need to use those

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out. No.

3️⃣ Does it include tests, if possible? Some tests were modified as needed for different behavior on 1.x and 2.x

4️⃣ Any drawbacks? Possible breaking changes? None detected nor expected.

5️⃣ Mark the following tasks as done:

• [X] Checked the codebase to ensure that your feature doesn't already exist.
• [X] Take note of the contributing guidelines.
• [X] Checked the pull requests to ensure that another person hasn't already submitted a fix.
• [X] Added tests to ensure against regression.

6️⃣ Thanks for contributing! 🙌

[saulens22]

Yes, there are no major differences between 1.x and 2.x. I've been using this workaround in composer.json for months now:

"require": {
    "phpoffice/phpspreadsheet": "^2.1.0",
},
"provide": {
    "phpoffice/phpspreadsheet": "1.29"
},

Will test this pull request too, but so far it seems good!

[underdpt]

I think I hit one compatibility issue:

Declaration of Maatwebsite\Excel\DefaultValueBinder::bindValue(PhpOffice\PhpSpreadsheet\Cell\Cell $cell, $value) must be compatible with PhpOffice\PhpSpreadsheet\Cell\DefaultValueBinder::bindValue(PhpOffice\PhpSpreadsheet\Cell\Cell $cell, mixed $value): bool 

[JamesFreeman]

Is there any update on this @patrickbrouwers?

[patrickbrouwers]

No update, as I mentioned in previous PR attempts and questions. This will be done in the 4.0 release which has no eta

[JamesFreeman]

Understood, thanks for the update. :)

[emargareten]

https://github.com/advisories/GHSA-ghg6-32f9-2jp7 https://github.com/advisories/GHSA-wgmf-q9vr-vww6

[roc26002w]

I have same compatibility issue:

Declaration of Maatwebsite\Excel\DefaultValueBinder::bindValue(PhpOffice\PhpSpreadsheet\Cell\Cell $cell, $value) must be compatible with PhpOffice\PhpSpreadsheet\Cell\DefaultValueBinder::bindValue(PhpOffice\PhpSpreadsheet\Cell\Cell $cell, mixed $value): bool 

[geneowak]

No update, as I mentioned in previous PR attempts and questions. This will be done in the 4.0 release which has no eta

@patrickbrouwers given that there are two reported vulnerabilities with the version being used currently, is it possible to just put in a patch for this package while the rest wait for v4?

The cross-site scripting vulnerability is considered moderate (5.4/10) but the bug where bypassing the filter allows an XXE-attack is classified with a high severity (8.8/10) and is only patched in v2.2.1 and above...

Is there some work required to get this fix in? we can help out with that.

[patrickbrouwers]

We'll first see if the backport https://github.com/PHPOffice/PhpSpreadsheet/pull/4154 gets merged

[prialgauskaslt]

Solution if your pipelines depend on this to be resolved. It seems it will not be done so soon. So just use proper composer.config.audit.ignore like this in your composer.json:

    "config": {
        "audit": {
            "ignore": {
                "CVE-2024-45048": "https://github.com/SpartnerNL/Laravel-Excel/pull/4164",
                "CVE-2024-45046": "https://github.com/SpartnerNL/Laravel-Excel/pull/4164"
            }
        }
    },

[kieranbarlow]

Do we have any progress on this?

[ultrono]

Kinda crazy this is ongoing IMO. I had a pretty important app. using this package with two exports. Decided to migrate them to a simple .CSV file. Unfortunately I couldn't get the above composer.json solutions working.

[prialgauskaslt]

Kinda crazy this is ongoing IMO. I had a pretty important app. using this package with two exports. Decided to migrate them to a simple .CSV file. Unfortunately I couldn't get the above composer.json solutions working.

Our CI/CD was stuck on composer audit —locked as it failed due to CVE. The solution I provided should be good for most checks during pipelines. Maybe if you’d share how your release/deployment is breaking due to active CVE, we can find a solution for that too. Sorry that I can’t provide more help without a context.

[ykchan97]

Ignoring the CVEs does not work for me because I have roave/security-advisories in require-dev (it will cause conflicts), I ended up using OP's forked repo temporarily with

"repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/mwikberg-virta/Laravel-Excel.git"
        }
    ]

So far it is working fine, will only switch back to main repo after the CVEs fixed (hope that we do not need to wait for v4 :)

[nuernbergerA]

https://github.com/PHPOffice/PhpSpreadsheet/pull/4154#issuecomment-2325731356

[ykchan97]

PHPOffice/PhpSpreadsheet#4154 (comment)

Yeah I tried this before, it conflicts with roave/security-advisories unfortunately

[patrickbrouwers]

Because v2 contains breaking changes (https://github.com/PHPOffice/PhpSpreadsheet/blob/master/CHANGELOG.md#breaking-change) that I want to check. Just don't have the time for it right now. The security fix will be backported to v1, that means all people using it including people on older php versions (v2 is >8.1) will get the fix.

[quaddy]

Yeah I tried this before, it conflicts with roave/security-advisories unfortunately

Same problem here. Would be nice when this can be handled prioritized.

[patrickbrouwers]

It's being handled by phpspreadsheet: https://github.com/PHPOffice/PhpSpreadsheet/pull/4154#issuecomment-2326727459 Please be patient, it's open source, nobody is paying PhpSpreadsheet to handle this with priority or even having to do the backport.

[nuernbergerA]

PHPOffice/PhpSpreadsheet#4154 (comment)

Yeah I tried this before, it conflicts with roave/security-advisories unfortunately

you need to disable/remove roave/security-advisories till its updated but at least you have a secure version installed

[patrickbrouwers]

Laravel Excel now requires 1.29.1 which has the security fix

[PanchoDP]

@patrickbrouwers Thank you!!!!

[stale[bot]]

This bug report has been automatically closed because it has not had recent activity. If this is still an active bug, please comment to reopen. Thank you for your contributions.