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.

Sign up for GitHub

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

Jump to bottom

Problem using serialized spreadsheet instance - static private members become null causing crash #1741

Closed
jkniftongh opened this issue Dec 4, 2020 · 1 comment · Fixed by #4326
Closed

Problem using serialized spreadsheet instance - static private members become null causing crash #1741

jkniftongh opened this issue Dec 4, 2020 · 1 comment · Fixed by #4326

Comments

@jkniftongh
Copy link

This is:

- [x] a bug report
  • We think this is a bug, but if serialization is not explicitly supported/envisioned - it could be considered a feature request

What is the expected behavior?

After loading complex spreadsheets for processing - we serialize the Spreadheet instance and save it in a cache for performance (as the raw load is taking a few seconds depending on the sheet and we are trying to keep the execution time down for a microservice call) - the unserialize is very quick.

What is the current behavior?

This used to work, however when we went from an older version to 1.14.1 the behaviour changed as an instance of Calculation is on the Spreadsheet instance, which when loaded normally from loading a SS file - it populated the private static member on the Calculation class "private static $referenceHelper" in the constructor [ self::$referenceHelper = ReferenceHelper::getInstance(); ].
The problem is that after serialized and then the serialized version is unserialized back into objects - the class instance of Calculation is there inflated on spreadsheet instance - so it uses it - BUT that instance has self::$referenceHelper as null, as serialize does not touch statics and constructors are not invoked for unserialize - the class however assumes the instance has the private static member populated.

This results in a break when a get calculated value call on a cell results in the evaluateDefinedName method being invoked - it breaks on :
"$definedNameValue = self::$referenceHelper->updateFormulaReferencesAnyWorksheet( ..."

In our application we have applied a workaround which checks is_null immediately prior and instantiates it if needed. To keep us moving forward.

What are the steps to reproduce?

  1. Load a spreadsheet with a named range in a formula on a cell
  2. Serialize the ss and save out (we do to a cache folder)
  • new session
  1. Load the ss from cached serialized object from file using unserialize
  2. get calculated value on cell with formula

Which versions of PhpSpreadsheet and PHP are affected?

We noticed it when we moved to 1.14.1 and is still a problem in 1.15.0
@MarkBaker
Copy link
Member

Feature request; PHPSpreadsheet has never officially supported serialization, and PHPExcel only ever supported it in the earliest versions

@oleibman oleibman reopened this Jan 26, 2025
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Jan 26, 2025
@oleibman
Allow Spreadsheet Serialization
8af9963 
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 issue Jan 26, 2025
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow Spreadsheet Serialization oleibman/PhpSpreadsheet
3 participants
@MarkBaker @oleibman @jkniftongh