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

Xlsx Reader parse quotePrefix attribute error #3435

Closed
4 of 11 tasks
littleylv opened this issue Mar 6, 2023 · 4 comments · Fixed by #3438
Closed
4 of 11 tasks

Xlsx Reader parse quotePrefix attribute error #3435

littleylv opened this issue Mar 6, 2023 · 4 comments · Fixed by #3438

Comments

@littleylv
Copy link

littleylv commented Mar 6, 2023

This is:

What is the expected behavior?

Load quotePrefix Correctly

What is the current behavior?

What are the steps to reproduce?

Source code: https://github.com/PHPOffice/PhpSpreadsheet/blob/master/src/PhpSpreadsheet/Reader/Xlsx.php#L621

test code:

<?php
$str = '<cellXfs count="2">
<xf numFmtId="0" fontId="0" fillId="0" borderId="0" pivotButton="0" quotePrefix="0" xfId="0" />
<xf numFmtId="0" fontId="0" fillId="0" borderId="0" pivotButton="0" quotePrefix="1" xfId="0" /></cellXfs>';
$xml = new \SimpleXMLElement($str);
foreach ($xml->xf as $xfTag) {
    $xf = $xfTag->attributes();
    var_dump((bool) ($xf['quotePrefix'] ?? false)); // source code copied from Xlsx.php#L621
    var_dump($xf['quotePrefix']);
    var_dump((string)$xf['quotePrefix']);
    var_dump((bool)(string)$xf['quotePrefix']);
}

/*
the above code returns:
bool(true)
object(SimpleXMLElement)#7 (1) {
  [0]=>
  string(1) "0"
}
string(1) "0"
bool(false)

bool(true)
object(SimpleXMLElement)#6 (1) {
  [0]=>
  string(1) "1"
}
string(1) "1"
bool(true)
*/

the source xml has two xf tags with quotePrefix="0" and quotePrefix="1" but var_dump((bool) ($xf['quotePrefix'] ?? false)); both return true.
Should $xf['quotePrefix'] be converted to string first?

What features do you think are causing the issue

  • Reader
  • Writer
  • Styles
  • Data Validations
  • Formula Calculations
  • Charts
  • AutoFilter
  • Form Elements

Does an issue affect all spreadsheet file formats? If not, which formats are affected?

Xlsx

Which versions of PhpSpreadsheet and PHP are affected?

PhpSpreadsheet: All
PHP: I am using 7.4.33

@oleibman
Copy link
Collaborator

oleibman commented Mar 6, 2023

I think that you are theoretically correct. Excel omits quotePrefix rather than including quotePrefix="0"; PhpSpreadsheet does the same. So, despite the theoretical exposure, we are probably okay. Which does not mean that it shouldn't be fixed - do you have an example where this is causing a problem? This particular code example occurs a second time in the module where you found it, and fortunately does not also occur in Reader/Xlsx/Styles.

@littleylv
Copy link
Author

Excel omits quotePrefix rather than including quotePrefix="0"

@oleibman You are right! My mistake.

I'm using PhpSpreadsheet for a long time without error until dealing with a file download from Amazon Sellercentral:

ManifestFileUpload_Template_MPL.xlsx

It uses quotePrefix="0" in xl/styles.xml and after I load and save it, all cells has a ':

include 'vendor/autoload.php';
$inputFileName = '/path/to/ManifestFileUpload_Template_MPL.xlsx';
$spreadsheet = \PhpOffice\PhpSpreadsheet\IOFactory::load($inputFileName);
$writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($spreadsheet);
$writer->save('/path/to/ManifestFileUpload_Template_MPL-new.xlsx');

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Mar 7, 2023
Fix PHPOffice#3435. Mis-parsed attribute is not normally generated by Excel or PhpSpreadsheet, but some 3rd-party software (correctly) generates it.
@oleibman
Copy link
Collaborator

oleibman commented Mar 7, 2023

Thank you for the sample file. If you can test against PR 3438, please do so.

@littleylv
Copy link
Author

I have confirmed that PR 3438 works correctly with my ManifestFileUpload_Template_MPL.xlsx

oleibman added a commit that referenced this issue Mar 7, 2023
* Correct Xlsx Parsing of quotePrefix="0"

Fix #3435. Mis-parsed attribute is not normally generated by Excel or PhpSpreadsheet, but some 3rd-party software (correctly) generates it.

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

Successfully merging a pull request may close this issue.

2 participants