Prevent XEE by hiding custom entities by using single quote to
declare a non-UTF-8 encoding.
XML standard, https://www.w3.org/TR/xml/#NT-EncodingDecl, allows single
quote to declare encoding, but we did not support it. Instead, we
incorrectly fell back on the default of UTF-8. That incorrectly kept the
XML as non-UTF-8, and thus prevented our regexp-based custom entity
detection mechanism to work.
Fix#4099. Ods Reader was expecting there to always be `header-style` and `footer-style` tags when `page-layout` tag is present, but these need not exist. It seemed like there might be other exposures along this line in `readPageSettingStyles`; rather than waiting for a problem report to show up for each, the code is updated to use `->item(0)` in place of `[0]` when appropriate, and make use of the nullsafe `?->` operator introduced with Php8.
Fix#804, opened in Dec. 2018, and closed as stale in Feb. 2019, and which I have re-opened to be closed properly by this PR. Better late than never, I suppose. A third party generated an ODS spreadsheet which PhpSpreadsheet could not read. By way of explanation, the xml in the file contained lots of whitespace between tags, which is wonderful for those humans among us who have to analyze it; but PhpSpreadsheet was not prepared for it. It is now.
Fix#4081. Ods Reader was not reading entire contents of comment. On further inspection, Ods Writer also was not handling comments completely correctly. Ods comments are recorded as `text:p` children of `office:annotation` elements. A newline is inserted between successive `text:p` elements. The `text:p` element itself can have as descendants (at least):
- raw text
- `text:span` elements
- `text:line-break` elements, which also causes the insertion of a newline
Ods Writer is changed to use a single `text:p` with multiple span/linebreak elements. Ods Reader is changed to process in their entirety either that form, or multiple `text:p` elements. Styling of the individual elements of the comment is permitted in Ods. That has not been supported till now by PhpSpreadsheet, and this PR will not address that situation - Ods Reader hast little style support, and this would hardly be the most urgent case where it is missing.
Fix#2581 (not obvious - see next paragraph for explanation). This continues the work of PR #2902 (and also PR #3467) to have errors propagated through function calculations rather than treating them as strings. All text functions, and the concatenation operator, are addressed in this PR.
In the original issue, the spreadsheet being loaded uses the result of an unimplemented function as an argument to another function. When `getCalculatedValue` is used on the cell in question, the result is returned as `#VALUE!`. If the cell had just contained a function call to the unimplemented function, getCalculatedValue would have recognized the situation and returned oldCalculatedValue as the result. Not perfect, but good enough most of the time. User would like oldCalculatedValue returned here as well, which seems like a reasonable request.
PhpSpreadsheet always returns `#Not Yet Implemented` as the result for a function which it knows about but which is not yet implemented. That is the key to the `Cell` class being able to substitute oldCalculatedValue in the first place. However, in order to do that for the issue in question, that result has to be propagated to any functions for which the result is an argument. I don't want to add unimplemented to the list of known error codes, but I am willing to add a parameter to `ErrorValue::isError` to indicate whether that value should be considered an error (default is "no").
The first use of that new parameter would be by the text functions. They go through a common Helper routine, so it is pretty easily implemented. And, as it turns out, most of the text functions do not currently propagate errors, e.g. if A1 results in a value error, `=LEFT(A1,2)` will result in `#V` rather than `#VALUE!`. With this PR, they will now be handled correctly.
This change is extracted from PR #2787 by @MarkBaker. That change mostly deals with array functions, and that part will be superseded by PR #3962. However, this part of 2787 is not included in 3962.
Fix#1036 (closed as stale in 2019 and just reopened). Excel spreadsheets can have either of 2 base dates, 1900 or 1904, and the numeric value of any date cells will vary depending on which base date is in use. PhpSpreadsheet has, till now, handled that as a static property of Shared/Date. This does not work well if two spreadsheets with different base dates are open simultaneously. The code is changed to store the base date as a property of the spreadsheet when an Xls/Xlsx spreadsheet is loaded, and use that property when saving an Xls/Xlsx spreadsheet. Any call to `getCalculatedValue` or `getFormattedValue` will temporarily set the Shared/Date value to that of the spreadsheet, and restore it at completion. In order to avoid a BC break, the Xls and Xlsx readers will continue to populate the Shared/Date value as before.
Fix#4063. Fix#1560. Fix#1293. PhpSpreadsheet is not accounting for an empty string in Xlsx sharedStrings.xml.The code which parses it in Reader/Xlsx looks for a `t` or `r` tag descending from `si`, but, in this case, the tag is coded as `<si/>`, with neither t nor r tag descending. An else clause is added to set the string to empty string in this case.
I was surprised that this had not turned up before, and a search through the archives found at least 2 earlier reports from 4 years ago. Those had been marked stale; the stale indicator is removed, and the issues are re-opened, to be closed when this PR is merged.
Fix#4049. Some possible options were not included for read or write. In addition, although it isn't well documented, it appears that 2-color scale always has 2 cvfo entries in Xml in order minimum/maximum, and 3-color scale always has 3 entries in order minimum/midpoint/maximum.
Xls Reader processing Conditionals interferes with the previously established SelectedCells. Make sure that value is restored.
StopIfTrue should always be set for Xls spreadsheet.
Set NoFormatSet to true unless any of Font, Fill, or Borders is specified in Conditional Style.
In my notes for PR #3372, I mentioned that I could not include some Xls tests because of errors in the software at that time. This PR fixes those errors, so I am adding the missing test, and making the equivalent Xlsx test more comprehensive.
Fix#3982. A number of issues submitted about Xlsx read performance have a common theme, namely that row 1,048,576 and a few rows before it are defined in the worksheet Xml with no cells attached to them. These might be the work of a third party product. While these extraneous rows do not cause any problems for the cells that are actually used on the worksheet, they can lead to excessive memory use. This PR provides an option for the application to ignore rows with no cells when loading.
Recent changes to the load logic had already made a significant difference to memory consumption and load time. For the spreadsheet attached to issue 3982, which had caused out-of-memory errors on the user's system, peak memory usage was already reduced to 40-odd MB. With the new option, this is drastically reduced again, to just over 9MB. Specifying the new option is very easy:
```php
$reader->setIgnoreRowsWithNoCells(true);
```
Note that there are cases where you might not want this (non-default) behavior. For example, if you set a row height on a row with no cells, the height would be lost with this option. Unfortunately, the extraneous row definitions in the problematic spreadsheets claim to have a custom height, so I can't just use "no custom row styles" as an additional filter.
While researching another problem, I noticed that font color was not working as expected for Xls Conditional Formats, at least not when a "non-standard" color is used. In such cases, the color might wind up being rendered as black. The reason is as follows. Xls Writer includes a color palette which is dynamically generated from the (non-Conditional) styles used in the workbook. Any colors used in the workbook are indexes to this dynamic palette. However, Conditional colors use a static palette found in class ColorMap to determine the index, so the determination of index will often not find a match, and, if a match is found, it is not necessarily correct. (Also, the ColorMap method was case-sensitive and needs to be insensitive.)
In order to correct this, the `addColor` method in Xls Writer Workbook needs to be accessible to the Conditional logic which is found in Xls Writer Worksheet. This is accomplished by passing the Workbook in the Worksheet's constructor, and changing the method to public, and changing Conditional Font to use this method rather than ColorMap.
The logic for Conditional Fill colors is similarly changed. Although Xls Conditional Fill has appeared to just not work, I was finally able to figure out the problem. Excel Xls Conditional Fill with fill type Solid requires that the fill color be specified as startColor, and that endColor be omitted. Our conditional samples used endColor, and are now changed to use startColor instead; the same is true for our online documentation, and for some tests. Xlsx continues to work as expected, and now Xls does at least some of the time. If the condition is one that Excel Xls does not recognize (e.g. cell contains), it will, of course, not work. A surprising situation that also doesn't work is the use of ISODD or ISEVEN in formulas. Those are "add-in functions" which are handled differently than other functions, and I'm not sure how to support them. I will document this in issue #3403.
Samples 08_Conditional_Formatting(_2) had produced corrupt Xls versions. This turned out to be because the code was using hash codes to avoid having to write out duplicate conditionals; this is often a good idea, but not in this case. Allowing the duplicates fixes the corruption problem.
Conditional Border colors also ought to figure in this change, but the current code does not support Border colors, and I have not yet been able to figure out how to implement it (BIFF format can be very messy to figure out).
With this change, I could delete ColorMap altogether. However, it is a public class with a static public method, so maybe someone is using it for a purpose I'm not familiar with. I will just deprecate it.
Fix#3995. Fix#866. Fix#1681. Php DOM loadhtml defaults to character set ISO-8859-1, but our data is UTF-8. So Html Reader alters its html so that loadhtml will not misinterpret characters outside the ASCII range. This works for UTF-8, but breaks other charsets. However, loadhtml uses the correct non-default charset when charset is specified in a meta tag, or when the html starts with a BOM. So, it is sufficient for us to alter the non-ASCII characters only when (a) the data does not start with a BOM, and (b) there is no charset tag.
This will allow us to use:
- UTF-8 files or snippets without BOM, with or without charset
- UTF-8 files with BOM (charset should not be specified and will be ignored if it is)
- UTF-16 files with BOM (charset should not be specified and will be ignored if it is)
- all charsets which are ASCII-compatible for 0x00-0x7f when the charset is declared. This applies to ASCII itself, many Windows and Mac charsets, all of ISO-8859, and most CJK and other-language-specific charsets.
We cannot use:
- UTF-16BE or UTF-16LE declared in a meta tag
- UTF-32, with or without a BOM (browser recommendation is to not support UTF-32, and most browsers do not support it)
- unknown (to loadhtml) or non-ASCII-compatible charsets (EBCDIC?)
I will note that the way I detect the `charset` attribute is imperfect (e.g. might find it in text rather than a meta tag). I think we'd need to write a browser to get it perfect. Anyhow, it is about the same as XmlScanner's attempt to find the `encoding` attribute, and, if it's good enough there, it ought to be good enough here.
Fix#3730. File workbook.xml.rels in test file specified absolute addressing for shared strings, styles, and worksheets. Previous changes had addressed absolute addressing for worksheets, but shared strings and styles had not yet been addressed.
Fix#3909. SUMPRODUCT is mishandling multi-row ranges. In Calculation/Calculation, `checkMatrixOperands` will often resize its operands. When it does so, it needs to recalculate the dimensions of each. This fixes the reported problem.
Likely cause was PR #3260. That ticket noted the poor coverage of the code being replaced. Tests of the problem in this ticket were absent and are now added. Despite this, I note that `resizeMatricesShrink` is virtually uncovered, and `resizeMatricesExpand` has substantial gaps in its coverage. I have covered some, but not all, of the Expand gaps. I am struggling to come up with examples to fill its remaining gaps and those for Shrink. However, I will merge this fix in about a week even if I don't succeed.
Fix#3863. Data Validation default operator is `between`. When Excel writes out a data validation item, it may omit the operator. Xlsx reader will therefore initialize operator to null string. Issue indicates that user wants `between` returned for `getOperator`. A more serious problem is that `isValid` method does not handle this situation correctly. Data Validation is changed to set Operator to the default value if an attempt is made to set it to null string.
Fix#3687. Worksheet methods insertNewRowBefore and insertNewColumnBefore call ReferenceHelper insertNewBefore. That function fills in "missing" cells with null values. However, for boundaries, it uses getHighestRow and getHighestColumn. It should be sufficient to use getHighestDataRow and getHighestDataColumn. When there is a big gap between getHighest... and getHighestData..., this can result in a big increase in memory usage, and in file space when saving the spreadsheet. New test InsertTest demonstrates the problem by populating a worksheet with cells A1:D5 (so highestDataRow is 5), but also setting row 1000 to invisible (so highestRow is 1000).
The major part of the change is in ReferenceHelper::insertNewBefore, which will now use getHighestData... for its boundaries when filling in the missing cells. Changes of less impact are made to duplicateStylesByColumn and duplicateStylesByRow so that cells which don't yet exist are not created unless the style that will be applied is not the workbook default style.
As for reducing the file size, Writer/Xlsx/Worksheet is changed so that cells whose value is null or null-string and which use the workbook default style are not written to the output spreadsheet. This requires some changes to existing test ReadBlankCellsTest; I don't think the difference should matter to the end-user.
Fix#3833. Among the options for chart axes in Excel are the ability to show labels as multiples of specific powers of 10, with or without an explanatory label. The Excel spec seems to indicate that that you can customize the multiples to other values, but I don't see how to do that in Excel, so that can be a project for another day if someone figures out how. The label title can also be styled; that is not part of this PR, but I will look into it in future. For now, this PR supports the use of multiples with or without labels.
The axes can also be displayed on a logarithmic scale (any base between 2 and 1000). This PR supports that.
Fix#3807. Function attempts to rationalize `..` in filenames in a way that normally works just fine. Reporter notes that at least one of the filenames that will be analyzed when a spreadsheet is read can be maliciously altered in a manner which does not harm Excel when reading the file, but which puts PhpSpreadsheet into a loop. This PR fixes the problem.
* Xlsx Support Flipping of Image
Fix#731. Opened over 5 years ago, probably the second oldest problem I've worked on. Images attached to an Xlsx spreadsheet can be rotated, which is supported by PhpSpreadsheet. They can also be flipped along their horizontal and/or vertical axes, and that has not been supported. This PR adds that support.
* Update CHANGELOG.md
* Address Some Chart Problems
Fix#3767. The basic problem in that issue was user error, but it exposed a couple of problems in code which are being addressed.
When a spreadsheet with charts is loaded without the includeCharts option and then saved, a corrupt spreadsheet is created. I believe this has been the case for quite some time. Nothing in the test suite covers this scenario. It is, in fact, a difficult thing to test, since the problem is exposed only when the file is opened through Excel. The specific problem is that a rels file generated by the output writer continues to refer to the drawing file which described the chart, but that file is not included (and not needed) in the output spreadsheet. The resolution is kludgey. The information that the file will not be needed is not available when the rels file is written. But, when it comes time to write the drawing file, it is known whether the rels file included it. So, if nothing else has caused the file to be generated, it is written out as a basically empty xml file after all. This solves the problem, but I will continue to search for a less kludgey solution. This solution is, at least, testable; if a different solution is applied later on, the test being introduced here is likely to break so a new one will be needed.
When the provided spreadsheet is loaded with the includeCharts option and then saved, an error is exposed in processing the Chart Title caption when it doesn't exist. The change to Writer/Xlsx/Chart is simple and clearly justifiable. What is peculiar is that the error does not arise with release 1.29, but does arise with master. It is not at all clear to me what has changed since the release to expose the error - the code in question certainly hasn't changed. It is difficult to isolate changes because of the extensive number of changes following on the elimination of Php7.4 as a supported platform.
The provided spreadsheet is unusual in at least two senses. When opened in Excel, it will show a clearly default value for the chart title, namely 'Chart Title'. I cannot find anything in the xml corresponding to that text. Since I have no idea why Excel is using that title, I will not try to duplicate its behavior, so that loading and saving the provided spreadsheet will omit the chart title. I will continue to investigate.
The other sense in which it is unusual is that it includes some style files in the same directory as the chart. I doubt that PhpSpreadsheet looks at these. The styling after load and save seems to mostly match the original, although there is at least one color in the graph which does not match. I imagine it would be pretty complicated to formally support these files.
* Unused Assignment
* Slk Shared Formulas
Fix#2267. The Slk format has a way to express a "shared formula", but the Slk reader does not yet understand it. Thanks to @SheetJSDev for documenting the problem and pointing the way towards a solution. It has taken a long time to get there. Part of the problem is that I have not been successful in getting Excel to use this type of construction when saving a Slk file. So I have resorted to saving a Slk file where shared formulas *could* be used, and then editing it by hand to actually use them. It would not surprise me in the least to have neglected one or more possible ways to specify a shared formula; but, at least the issue as documented is resolved, and if new issues arise, we'll probably be in better shape to deal with them.
* Update CHANGELOG.md
* Theme File Missing But Referenced in Spreadsheet
Fix#3770. A rels file points to a non-existent theme file in the spreadsheet. In other similar cases (e.g. PR #3771), Excel opens such a spreadsheet, but with an error pop-up. Not so with this file; it just opens the spreadsheet without the pop-up. PhpSpreadsheet will now account for this unusual situation as well.
* Update CHANGELOG.md
* Added Conditional Formatting: ColorScale for Xlsx
* Add Reader Support, Tests, Sample
Also correct Phpstan and phpcs problems.
* Update cond08_colorscale.php
* Improve Coverage
* More Coverage Improvements
* Use StyleReader for Colors for ColorScale and DataBar
The implementation of DataBar looks for an rgb attribute, but the color may be provided via theme attribute instead. The initial implementation for ColorScale did the same. Change both to use the existing code in Reader\Xlsx\Styles to parse the color.
* Change Some Doc Blocks to Type Declarations
---------
Co-authored-by: oleibman <10341515+oleibman@users.noreply.github.com>
* fix: Load tables when flag IReader::READ_DATA_ONLY is used
* Add test for table read when IReader:::READ_DATA_ONLY is used
---------
Co-authored-by: Kevin Verschaeve <kevin.verschaeve@exotec.com>
* Add iterateOnlyExistingCells to Constructors
Fix#3721. That issue can already be handled, but requires several statements when one ought to suffice. Adding an extra parameter to the RowCellIterator and ColumnCellIterator constructors is useful, easy, and becomes especially practical now that our supported Php releases all support named parameters (see new test Issue3721Test).
* Update Column.php
* Update .php-cs-fixer.dist.php
* Update Row.php
* Update Column.php
* Update ByColumnAndRowTest.php
* Unexpected Namespacing in rels File
Fix#3720. Third-party product created a spreadsheet which PhpSpreadsheet could not read because of unexpected namespacing in workbook.xml.rels.
The file which demonstrated the problem was attached to #3423, however I do not believe it was related to the original problem. Nevertheless, the original issue specifically called out Protection, so I put some Protection tests in the validation test for the fix. In doing so, I found that Style/Protection is particularly confusing. Its properties will often have the value `inherit`, which isn't all that helpful; and, even when the `locked` value is `protected`, the cell won't actually be locked unless the sheet is protected as well. The `hidden` property is even more obscure - it applies only to formulas, and refers to hiding the property on the formula bar, not in the cell. I have added methods `isLocked` and `isHiddenOnFormulaBar` to `Cell`. I corrected the docs to explain this. And, as long as I was looking at the docs, I corrected some examples to use `getHighestDataRow/Column` rather than `getHighestRow/Column`, a frequent problem for users (e.g. #3721).
As a side note, the change to Cell.php is my first use of the nullsafe operator. This is one of many new options available now that we require Php8.0+.
* Minor Simplifications
* Scrutinizer
It's being silly again. In many tests, we test a variable for non-null, then use that variable later and Scrutinizer knows it's not null. Not here. Oh well.
* Add Methods
Test if protected without allocating cell if it doesn't exist.
* ListWorksheetInfo/Names for Html/Csv/Slk
Fix#3706. ListWorksheetInfo is implemented for all Readers except Html. For most (not all), ListWorksheetInfo is more efficient than reading the spreadsheet. I can't think of a way to make that so for Html, but that shouldn't be a reason to leave it unimplemented.
ListWorksheetNames is not implemented for Html, Csv, or Slk. It isn't terribly useful for those formats, but that isn't a reason to omit it. The requester's use case consists of using IOFactory to create a reader for a file of unknown format and determining the first sheet name. That seems legitimate, but it is currently not possible without extra user code if the file is Html, Csv, or Slk; this PR will make it possible.
When Excel opens a Slk or Csv file, the sheet name is based on the file name. PhpSpreadsheet does this for Slk, but it uses a default name for Csv. I am not interested in creating a break for that behavior, but I have added a new boolean property `sheetNameIsFileName` with a setter to Csv Reader. The requester actually mentioned that possibility in our discussion, although it is not essential to the request.
As an adjunct to the issue, the requester wishes to use the worksheet name in `setLoadSheetsOnly`. That is already possible for Html, Csv, and Slk, but that particular property is ignored for those formats. I do not see a reason to change that behavior. This treatment is now explicitly noted in the documentation for property `loadSheetsOnly`.
There had been no tests for what happens when `loadSheetsOnly` is specified but no sheets match the criteria for the formats for which this makes sense (Xlsx, Xls, Ods, Gnumeric, Xml). The behavior was not consistent - some formats threw an Exception while others continued with a single empty worksheet. All cases attempt to set the active sheet, and they will now all throw identical Exceptions when they attempt to do so in this situation. Tests are added for each.
There also had been no tests for `loadSheetsOnly` returning more than one sheet. One is added.
* Update LoadSheetsOnlyTest.php
Add strict types to this new test, consistent with work being done in PR #3718.
* Update LoadSheetsOnlyTest.php
Add strict types to this new test, consistent with work being done in PR #3718.
oleibman
Adrien Crivelli
redtailmatt
jenasnic
Kevin Verschaeve