Invalid content in VML comments #170

emeraldjava · 2017-06-12T09:32:26Z

What are the steps to reproduce?

Hi,

I'm trying to load a excel file using the load function via the code below. I want to be able to write data to the workbook

$spreadsheet = PhpOffice\PhpSpreadsheet\IOFactory::load("BHAA.Race.Master.xlsm");

see https://github.com/emeraldjava/bhaa-racemaster/blob/master/BHAA.Race.Master.xlsm for the target file.

What is the current behavior?

I get this error when loading, which indicated there are invalid XML details in the VML comments

[12-Jun-2017 09:14:36 UTC] PHP Warning:  simplexml_load_string():  &lt;/v:shape&gt;&lt;/xml&gt; in /home/pauloconnell/projects/github/bhaawp/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Reader/Excel2007.php on line 1354
[12-Jun-2017 09:14:36 UTC] PHP Warning:  simplexml_load_string():                  ^ in /home/pauloconnell/projects/github/bhaawp/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Reader/Excel2007.php on line 1354
[12-Jun-2017 09:14:36 UTC] PHP Fatal error:  Call to a member function registerXPathNamespace() on boolean in /home/pauloconnell/projects/github/bhaawp/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Reader/Excel2007.php on line 1355

Looking at the code in the Excel2007 file

// Loop through VML comments
foreach ($vmlComments as $relName => $relPath) {
    // Load VML comments file
    $relPath = \PhpOffice\PhpSpreadsheet\Shared\File::realpath(dirname("$dir/$fileWorksheet") . '/' . $relPath);
    $vmlCommentsFile = simplexml_load_string(
        $this->securityScan($this->getFromZipArchive($zip, $relPath)),
        'SimpleXMLElement',
        \PhpOffice\PhpSpreadsheet\Settings::getLibXmlLoaderOptions()
    );
    $vmlCommentsFile->registerXPathNamespace('v', 'urn:schemas-microsoft-com:vml');

because the 'simplexml_load_string' fails, the subsequent '$vmlCommentsFile->registerXPathNamespace()' is not a valid reference.

Which versions of PhpSpreadsheet and PHP are affected?

I'm using php 5.6.30.

My composer config has

"require": {
  "phpoffice/phpspreadsheet": "dev-master"
}

Has anybody seen this issue before or perhaps can suggest a fix. I'm happy to create a push request but am unsure how to best handle this situation.

The text was updated successfully, but these errors were encountered:

PowerKiKi · 2017-06-12T17:05:02Z

You are using an old version of PhpSpreadsheet, start by upgrading and see if it still happens.

emeraldjava · 2017-06-13T09:30:26Z

@PowerKiKi - I updated the version to 'phpoffice/phpspreadsheet-->dev-develop' but the issue remains.

Having investigated the vmlDrawing.xml files of the workbook, we have a number of macro buttons where the text message is being formatted with html   elements. These formatting statements are not contained in CDDATA blocks, so the php 'simplexml_load_string()' fails. I wondering is there an alternative xml processing method which is more flexible in terms of the xml structure which control the button formatting.

…port the which sheet contains the invalid xml.

PowerKiKi · 2017-06-20T09:45:44Z

It sounds like your file is incorrectly written. Are you able to reproduce the bug with a minimal file that is written with Excel 2016 ?

Fix PHPOffice#3125 (rejected previous PR PHPOffice#1181 as too risky, issue was also reported as PHPOffice#170). Vml file should be valid Xml, but Excel (possibly just Excel 2013) can generate unclosed ` ` tags, preventing Xlsx reader from reading file correctly. I believe a very narrowly targeted fix, changing ` ` to ` `, and only when reading vml files, probably mitigates the risk. I am going to leave this PR as a draft for a while to see if others disagree. Another reason to leave it as a draft is the absence of a decent test file. I am able to copy an existing test spreadsheet which contains a vml file, and add some text and the problematic tag, and I can confirm that it fails to load correctly without the fix but loads correctly with the fix. However, it would be better if I had a real file. I cannot figure out how to generate a file like this "naturally"; when I add a textbox to a spreadsheet, it is stored as a regular xml file, not as vml, in both Excel 365 and 2007. Also, the text that I added in the vml file doesn't show anywhere when I open the file in Excel, so I don't know whether my file is a normal case for this condition. And, incidentally, when I load the file in PhpSpreadsheet and save it, the textbox data is not in the resulting output file; this is clearly not a problem in the particular case I constructed, but I don't know if that's true in general. If not, that would be a different problem than the one I'm fixing, but I would prefer to resolve them both at the same time.

This is a replacement for draft PR PHPOffice#2455 and draft PR PHPOffice#3127. There is some useful commentary in those PRs which I have mostly, but not entirely, duplicated below. Fix PHPOffice#2396. Fix PHPOffice#1770. Fix PHPOffice#2388. A related problem is that the vml files used for the form controls sometimes contain invalid xml. Fix PHPOffice#3125 (rejected previous PR PHPOffice#1181 as too risky, issue was also reported as PHPOffice#170). Vml file should be valid Xml, but Excel can generate unclosed ` ` tags, preventing Xlsx reader from reading file correctly. I believe a very narrowly targeted fix, changing ` ` to ` `, and only when reading vml files, probably mitigates the risk. The sample file formscomments.xlsx which is part of this change shows this problem with `Button 1` on sheet `Forms`; the spreadsheet was created with Excel 365, so the problem is not restricted to Excel 2013 as originally reported. A comment on PR 3127 indicates that other tags might be involved, but, without a file demonstrating that, I will restrict this change to br tags for now. I am starting this out in draft status, and will probably leave it that way for some time. I'm not sure where we want to go with this. It fixes some problems, but in a limited manner, and creates some others. I'm not sure the pain of the others is balanced considering the limitations of the fix. If enough interest is generated as a result of this ticket being out there, we can proceed; if not, it probably isn't worth it. This fix allows form control elements to be read in and written out. It does not allow you to add such elements, nor even to locate them or determine their properties (so you can't modify or delete them). Although it handles reading and writing of sheets containing both form controls and comments, it will probably create a corrupt spreadsheet if you try adding a new comment to a sheet with form controls - probably quite difficult to solve. Cloning the sheet probably won't work either - probably easier than the other. It is conceivable that we want to add a new property to the Xlsx Reader which turns the reading of form elements on or off (default=off), so that negative effects will be limited to those who have explictly opted in. The change in its current form does not implement such a property. Because of its limitations, the change isn't really testable. As in some other recent installs, I have added a sample to demonstrate that it works correctly. As it turns out, if we have a worksheet which contains both form controls and comments (see formscomments.xlsx which is part of this PR), PhpSpreadsheet already creates a corrupt file when it tries to load and save the spreadsheet with such a worksheet. With this change, the file is saved without corruption. This tilts things in favor of proceeding. I'm still not ready, but this will be an important consideration. A sample file for issue PHPOffice#2621 illustrated a problem with shape files. Since they are involved here, I took a look at how the sample worked with this code. In master, and with this change, a corrupt file results. Fixing that is probably easier than the general problem of handling shape files, but it's an argument against moving this forward until the corruption problem can be addressed. Fix PHPOffice#2661. A template including checkboxes was leading to file corruption solved by this PR. Another argument for moving forward.

* WIP Limited Support for Form Controls V2 (ListBox, Buttons, etc.) This is a replacement for draft PR #2455 and draft PR #3127. There is some useful commentary in those PRs which I have mostly, but not entirely, duplicated below. Fix #2396. Fix #1770. Fix #2388. A related problem is that the vml files used for the form controls sometimes contain invalid xml. Fix #3125 (rejected previous PR #1181 as too risky, issue was also reported as #170). Vml file should be valid Xml, but Excel can generate unclosed ` ` tags, preventing Xlsx reader from reading file correctly. I believe a very narrowly targeted fix, changing ` ` to ` `, and only when reading vml files, probably mitigates the risk. The sample file formscomments.xlsx which is part of this change shows this problem with `Button 1` on sheet `Forms`; the spreadsheet was created with Excel 365, so the problem is not restricted to Excel 2013 as originally reported. A comment on PR 3127 indicates that other tags might be involved, but, without a file demonstrating that, I will restrict this change to br tags for now. I am starting this out in draft status, and will probably leave it that way for some time. I'm not sure where we want to go with this. It fixes some problems, but in a limited manner, and creates some others. I'm not sure the pain of the others is balanced considering the limitations of the fix. If enough interest is generated as a result of this ticket being out there, we can proceed; if not, it probably isn't worth it. This fix allows form control elements to be read in and written out. It does not allow you to add such elements, nor even to locate them or determine their properties (so you can't modify or delete them). Although it handles reading and writing of sheets containing both form controls and comments, it will probably create a corrupt spreadsheet if you try adding a new comment to a sheet with form controls - probably quite difficult to solve. Cloning the sheet probably won't work either - probably easier than the other. It is conceivable that we want to add a new property to the Xlsx Reader which turns the reading of form elements on or off (default=off), so that negative effects will be limited to those who have explictly opted in. The change in its current form does not implement such a property. Because of its limitations, the change isn't really testable. As in some other recent installs, I have added a sample to demonstrate that it works correctly. As it turns out, if we have a worksheet which contains both form controls and comments (see formscomments.xlsx which is part of this PR), PhpSpreadsheet already creates a corrupt file when it tries to load and save the spreadsheet with such a worksheet. With this change, the file is saved without corruption. This tilts things in favor of proceeding. I'm still not ready, but this will be an important consideration. A sample file for issue #2621 illustrated a problem with shape files. Since they are involved here, I took a look at how the sample worked with this code. In master, and with this change, a corrupt file results. Fixing that is probably easier than the general problem of handling shape files, but it's an argument against moving this forward until the corruption problem can be addressed. Fix #2661. A template including checkboxes was leading to file corruption solved by this PR. Another argument for moving forward. * Improved Sample File, and Documentation Add more realistic worksheet to spreadsheet. Document new feature, adding caveats to how it can be used.

PowerKiKi closed this as completed Jun 12, 2017

emeraldjava mentioned this issue Jun 13, 2017

Macro button text with new line characters emeraldjava/bhaa-racemaster#1

Open

emeraldjava pushed a commit to emeraldjava/PhpSpreadsheet that referenced this issue Jun 13, 2017

PHPOffice#170 Handle if simplexml_load_string() returns false, and re…

8e390bc

…port the which sheet contains the invalid xml.

PowerKiKi mentioned this issue Jun 20, 2017

XSLM Workbook - macro logic is removed when file is written to. #172

Closed

oleibman mentioned this issue Oct 17, 2022

simplexml_load_string / Opening and ending tag mismatch in XLSX-Reader #3125

Closed

oleibman mentioned this issue Oct 19, 2022

WIP Xlsx Reader Vml File with Unclosed br Tags #3127

Closed

7 tasks

oleibman mentioned this issue Oct 20, 2022

Limited Support for Form Controls V2 (ListBox, Buttons, etc.) #3130

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Invalid content in VML comments #170

Invalid content in VML comments #170

emeraldjava commented Jun 12, 2017 •

edited by PowerKiKi

Loading

PowerKiKi commented Jun 12, 2017

emeraldjava commented Jun 13, 2017 •

edited by PowerKiKi

Loading

PowerKiKi commented Jun 20, 2017

Invalid content in VML comments #170

Invalid content in VML comments #170

Comments

emeraldjava commented Jun 12, 2017 • edited by PowerKiKi Loading

What are the steps to reproduce?

What is the current behavior?

Which versions of PhpSpreadsheet and PHP are affected?

PowerKiKi commented Jun 12, 2017

emeraldjava commented Jun 13, 2017 • edited by PowerKiKi Loading

PowerKiKi commented Jun 20, 2017

emeraldjava commented Jun 12, 2017 •

edited by PowerKiKi

Loading

emeraldjava commented Jun 13, 2017 •

edited by PowerKiKi

Loading