Skip to content

Explicitly prevent serialization of the Spreadsheet object #3199

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

Merged
merged 2 commits into from
Nov 23, 2022

Conversation

MarkBaker
Copy link
Member

This is:

- [ ] a bugfix
- [X] a new feature
- [ ] refactoring
- [ ] additional unit tests

Checklist:

Why this change is needed?

Because people try to do it, and then say that it's a bug when they can't; so an explicit exception when they try should stop them from raising it as an issue

@MarkBaker MarkBaker merged commit a3187a3 into master Nov 23, 2022
@MarkBaker MarkBaker deleted the Functionality-Prevent-Serialization branch November 25, 2022 13:59
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this pull request Jan 26, 2025
Fix PHPOffice#4324. Serialization was explicity forbidden by PR PHPOffice#3199. This was in response to several issues, and concern that the Spreadsheet object contained non-serializable properties. This PR restores the ability to serialize a spreadsheet. Json serialization remains unsupported.

Fix PHPOffice#1757, closed in Nov. 2023 but just reopened. At the time, Cell property `formulaAttributes` was stored as a SimpleXmlElement. Dynamic arrays PR PHPOffice#3962 defined that property as `null|array<string, string>` in the doc block. However, it left the formal Php type for the property as `mixed`. This PR changes the formal type to `?array`.

Fix PHPOffice#1741, closed in Dec. 2020 but just reopened. Calculation property `referenceHelper` was defined as static, and, since static properties don't take part in serialization, this caused a problem after unserialization. There are at least 3 trivial ways to deal with this - make it an instance property, reinitialize it when unserialized using a wakeup method, or remove the property altogether. This PR uses the last of those 3.

Calculation does have other static properties. Almost all of these deal with locale. So serialize/unserialize might wind up using a default locale when non-default is desired (but not necessarily required). If that is a problem for end-users, it will be a new one, and I will work on a solution if and when the time comes.

Static property `returnArrayAsType` is potentially problematic. However, instance property `instanceArrayReturnType` is the preferred method of handling this, and using that will avoid any problems.

Issue PHPOffice#932 also dealt with serialization. I do not have the wherewithal to investigate that issue. If it is not solved by this and the earlier PR's, I will have to leave it to others to re-raise it.

Spreadsheet `copy` is now simplified to use serialize followed by unserialize. Formal tests are added. In addition, I have made a number of informal tests on very complicated spreadsheets, and it has performed correctly for all of them.
@oleibman oleibman mentioned this pull request Jan 26, 2025
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant