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

CRASH load,save,load,save=Crash if images in footer #3126

Closed
2 tasks
alaindev opened this issue Oct 17, 2022 · 2 comments · Fixed by #3140
Closed
2 tasks

CRASH load,save,load,save=Crash if images in footer #3126

alaindev opened this issue Oct 17, 2022 · 2 comments · Fixed by #3140

Comments

@alaindev
Copy link

This is:

Hello

PHPSpreadsheet CRASHes when 2 consuccessif load->Save if images are in headerFooterImages

I have an Excel-1 file as a template

load Excel-1=>save to Excel-2

load Excel-2 =>save to Excel-Final = CRASH since footer image is no longer found in zip!


### What is the expected behavior?
No Exception

### What is the current behavior?
ThrowException in 

### What are the steps to reproduce?

Please provide a [Minimal, Complete, and Verifiable example](https://stackoverflow.com/help/mcve) of code that exhibits the issue without relying on an external Excel file or a web server:

<?php

require 'includes/modules/autoload.php';

// Create new Spreadsheet object
$spreadsheet = \PhpOffice\PhpSpreadsheet\IOFactory::load("DEBUG.xlsx");
			$filewriter = \PhpOffice\PhpSpreadsheet\IOFactory::createWriter($spreadsheet , "Xlsx");
			$filewriter->setIncludeCharts(true);
			$filewriter->save("EXCEL2.xlsx");

$spread2sheet = \PhpOffice\PhpSpreadsheet\IOFactory::load("EXCEL2.xlsx");
			$filewriter2 = \PhpOffice\PhpSpreadsheet\IOFactory::createWriter($spread2sheet , "Xlsx");
			$filewriter2->setIncludeCharts(true);
			$filewriter2->save("EXCELFINAL.xlsx"); // => CRASH if image in Footer

exit();
?>

### What features do you think are causing the issue

- [ ] Reader
- [ X] 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?
All
[DEBUG.xlsx](https://github.com/PHPOffice/PhpSpreadsheet/files/9802175/DEBUG.xlsx)
@oleibman
Copy link
Collaborator

This appears to be a problem of ancient vintage. The code above with the supplied file works with PHPExcel, and with PhpSpreadsheet through version 1.2.1. It fails starting with version 1.3.0 (June 9, 2018).

@alaindev
Copy link
Author

Right : tested current version is "phpoffice/phpspreadsheet": "^1.16",

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Oct 25, 2022
Fix PHPOffice#3126. A worksheet contained an image in its footer. It could be loaded and saved as another spreadsheet. However, if you tried to load and save that spreadsheet, PhpSpreadsheet would be unable to find the footer image and would therefore throw an exception.

This error was introduced a long time ago, in PhpSpreadsheet 1.3.0. The apparent cause of the problem was PR PHPOffice#435, sometime around June 2018. That change was very useful, but it had problems which exposed themselves only with a third generation copy. An additional contributor to the issue at hand was PR PHPOffice#1690 (December 2020), which again exposed itself with a third generation copy.

The issue from 1690 is easier to explain and deal with. It added a 'ps' suffix to printer settings resources in Xlsx Reader (to avoid name conflicts), but did not limit itself to a single addition (so subseqent generations would have multiple ps's). It also neglected to add the suffix in Reader/Xlsx/PageSetup.

As for 435, it loops through all the worksheet relationships, and uses the last that it finds as the base for header/footer drawings. It has been changed to use only the relationship whose `rId` matches the worksheet's `legacyDrawingHF` `rId`. It also needs a bit extra validation to make sure a drawing exists before adding it to its array of header/footer images. It also meant that Xlsx/Writer/Rels might write an entry with the same rId twice. I have also changed the header/footer image processing to be namespace aware (see PR PHPOffice#3137).
oleibman added a commit that referenced this issue Nov 2, 2022
* Generation3 Copy With Image in Footer

Fix #3126. A worksheet contained an image in its footer. It could be loaded and saved as another spreadsheet. However, if you tried to load and save that spreadsheet, PhpSpreadsheet would be unable to find the footer image and would therefore throw an exception.

This error was introduced a long time ago, in PhpSpreadsheet 1.3.0. The apparent cause of the problem was PR #435, sometime around June 2018. That change was very useful, but it had problems which exposed themselves only with a third generation copy. An additional contributor to the issue at hand was PR #1690 (December 2020), which again exposed itself with a third generation copy.

The issue from 1690 is easier to explain and deal with. It added a 'ps' suffix to printer settings resources in Xlsx Reader (to avoid name conflicts), but did not limit itself to a single addition (so subseqent generations would have multiple ps's). It also neglected to add the suffix in Reader/Xlsx/PageSetup.

As for 435, it loops through all the worksheet relationships, and uses the last that it finds as the base for header/footer drawings. It has been changed to use only the relationship whose `rId` matches the worksheet's `legacyDrawingHF` `rId`. It also needs a bit extra validation to make sure a drawing exists before adding it to its array of header/footer images. It also meant that Xlsx/Writer/Rels might write an entry with the same rId twice. I have also changed the header/footer image processing to be namespace aware (see PR #3137).

* Minor Change

I didn't like the way I performed one operation.

* Fix Test

Array index should not have been constant.
MarkBaker added a commit that referenced this issue Dec 21, 2022
### Added

- Extended flag options for the Reader `load()` and Writer `save()` methods
- Apply Row/Column limits (1048576 and XFD) in ReferenceHelper [PR #3213](#3213)
- Allow the creation of In-Memory Drawings from a string of binary image data, or from a stream. [PR #3157](#3157)
- Xlsx Reader support for Pivot Tables [PR #2829](#2829)
- Permit Date/Time Entered on Spreadsheet to be calculated as Float [Issue #1416](#1416) [PR #3121](#3121)

### Changed

- Nothing

### Deprecated

- Direct update of Calculation::suppressFormulaErrors is replaced with setter.
- Font public static variable defaultColumnWidths replaced with constant DEFAULT_COLUMN_WIDTHS.
- ExcelError public static variable errorCodes replaced with constant ERROR_CODES.
- NumberFormat constant FORMAT_DATE_YYYYMMDD2 replaced with existing identical FORMAT_DATE_YYYYMMDD.

### Removed

- Nothing

### Fixed

- Fixed handling for `_xlws` prefixed functions from Office365 [Issue #3245](#3245) [PR #3247](#3247)
- Conditionals formatting rules applied to rows/columns are removed [Issue #3184](#3184) [PR #3213](#3213)
- Treat strings containing currency or accounting values as floats in Calculation Engine operations [Issue #3165](#3165) [PR #3189](#3189)
- Treat strings containing percentage values as floats in Calculation Engine operations [Issue #3155](#3155) [PR #3156](#3156) and [PR #3164](#3164)
- Xlsx Reader Accept Palette of Fewer than 64 Colors [Issue #3093](#3093) [PR #3096](#3096)
- Use Locale-Independent Float Conversion for Xlsx Writer Custom Property [Issue #3095](#3095) [PR #3099](#3099)
- Allow setting AutoFilter range on a single cell or row [Issue #3102](#3102) [PR #3111](#3111)
- Xlsx Reader External Data Validations Flag Missing [Issue #2677](#2677) [PR #3078](#3078)
- Reduces extra memory usage on `__destruct()` calls [PR #3092](#3092)
- Additional properties for Trendlines [Issue #3011](#3011) [PR #3028](#3028)
- Calculation suppressFormulaErrors fix [Issue #1531](#1531) [PR #3092](#3092)
- Permit Date/Time Entered on Spreadsheet to be Calculated as Float [Issue #1416](#1416) [PR #3121](#3121)
- Incorrect Handling of Data Validation Formula Containing Ampersand [Issue #3145](#3145) [PR #3146](#3146)
- Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3136](#3137)
- Generation3 Copy With Image in Footer [Issue #3126](#3126) [PR #3140](#3140)
- MATCH Function Problems with Int/Float Compare and Wildcards [Issue #3141](#3141) [PR #3142](#3142)
- Fix ODS Read Filter on number-columns-repeated cell [Issue #3148](#3148) [PR #3149](#3149)
- Problems Formatting Very Small and Very Large Numbers [Issue #3128](#3128) [PR #3152](#3152)
- XlsxWrite preserve line styles for y-axis, not just x-axis [PR #3163](#3163)
- Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3137](#3137)
- More Detail for Cyclic Error Messages [Issue #3169](#3169) [PR #3170](#3170)
- Improved Documentation for Deprecations - many PRs [Issue #3162](#3162)
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