Xls Reader can read drawing offsetX and offsetY as float. However, Excel Xlsx (and PhpSpreadsheet) wants them only as int. This leads 20_Read_Xls and 30_Template to produce corrupt Xlsx files for any Php release. Change Xls Reader to treat values as int, eliminating the corrupt files.
Dependabot pushed Phpstan 1.4.4 and then 1.4.5 (PR #2548). Because of some incompatible error messages between the current and new release, changes are required in order to be compatible with current and new releases.
See issue #2315. It is nominally solved by PR #2312, but that PR is completely unsuitable for merging. This one-line change is a replacement for that PR.
As with many problems of this type, it is not clear how how to create a spreadsheet with this sort of harmless corruption in the wild. An example was supplied with the issue, and I have tested manually against it. However, the file is huge and not suitable for a formal unit test. I do not understand BIFF well enough to try and craft a suitable example on my own.
Co-authored-by: Adrien Crivelli <adrien.crivelli@gmail.com>
See issue #2239. Problem is dealt with at the source, by making sure that Reader Xls checks for use of 'GENERAL' rather than 'General'. There doesn't seem to be a reason to test in other places, or to test for other casing variants.
* Tweaks to Input File Validation
This started as a response to issue #1718, for which it is a partial (not complete) solution. The following changes are made:
- canRead can currently throw an exception. This seems wrong. It should just return true/false.
- Breaking change of sorts. When AssertFile encounters a non-existent or unreadable file, it throws InvalidArgumentException. This does not make sense. I have changed it to throw PhpSpreadsheet/Reader/Exception.
- Since the previous bullet item required changing of most of the Reader files anyhow, this is a good time to add explicit typing for canRead in the function signature rather than the DocBlock. Since all the canRead functions inherit from an abstract version in IReader, they all have to be changed simulatneously. Except for Xlsx and Ods, most of the Reader files are otherwise unchanged.
- AssertFile is changed to add an optional "zip member" parameter. It will check for the existence of an appropriate member in what is supposed to be a zip file. It is used by Xlsx and Ods.
- Verifying that a given file is a valid zip ought to be a feature of ZipArchive. Thanks to a particularly nasty bug in php/libzip (see https://bugs.php.net/bug.php?id=81222), it is unsafe to attempt to open a zero-length file as a zip archive. There is a solution, but it does not apply to all the PHP releases which we support, and isn't even necessarily supported on all the point versions of the PHP versions which we do support. I have coded up a manual test for "valid zip", with a comment pointing to the spec.
- In theory, tests now cover 100% of the code in Shared/File. In practice ... One of the tests require that chmod works properly, which is not quite true on Windows systems, so that test is skipped on Windows. Another test requires that php.ini uses a non-default value for upload_temp_dir (can't be overridden in application code), which is probably not the case when Github runs the unit tests, so that test is skipped when appropriate. I have run tests for both on systems where they are not skipped.
* Update File.php
* Scrutinizer Timeout
It's not actually timing out, it's just waiting for something to finish that finished ages ago. Making a meaningless comment change in hopes that will clear the jam. Not particularly hopeful.
* Use of passing flags with Readers to identify whether speacial features such as loading charts should be enabled; no need to instantiate a reader and manually enable it before loading any more.
This is in preparation for supporting new "boolean" Reaer/Writer features, such as pivot tables
* Use of passing flags with Writers to identify whether speacial features such as loading charts should be enabled; no need to instantiate a writer and manually enable it before loading any more.
* Update documentation with details of changes to the StringValueBinder
* Document Properties - Coverage and 32-bit-safe Timestamps
While researching an issue, I noticed that coverage of Document/Properties was poor. Further, the use of int timestamps will eventually lead to problems for 32-bit PHP (see issue #1826).
Coverage Changes:
- Many property types with no special handling are enumerated but not tested. These are removed, but will continue to function as before.
- Existing code theoretically allows property to be set to an object, but there is no means to read or write such a property, and, even if there were, I don't believe Excel supports it. Setting a property to an object will now be changed to a no-op (can throw an exception if preferred).
- Since the Properties object now has no members which are themselves objects, there is no need for a deep clone. The untested __clone method is removed.
- Large switch statements are replaced with associative arrays. Scrutinizer will like that.
- Coverage is now 100%.
<!-- end of coverage changes list -->
Timestamp Changes:
- Timestamps will be stored as int if possible, or float if not. This is, or will soon be, needed for 32-bit systems. Tests have been added for beyond-epoch dates, and run successfully with 32-bit.
- LibreOffice doesn't quite get the Created/Modified properties correct. These are written to the file as a string which includes offset from UTC, but LibreOffice ignores the offset portion when displaying them. Code had been generating these in UTC, but now generates them in default timezone, which should meet user's expectations.
<!-- end of timestamp changes list -->
Other Changes:
- Custom properties added to ODS Writer.
- Samples had not been generating any ODS files. One is now generated.
- Ods uses a single 'keywords' property rather than multiple 'keyword' properties.
- Breaking change - default company is changed to null string from Microsoft Corporation.
- Breaking change of sorts - PropertiesTest incorrectly tested a custom date property against a string, Reader/XlsxTest correctly tested against a timestamp converted to a string. PropertiesTest was defective, and will no longer work as coded; anyone using it as a model will likewise have a problem.
- PHP8.1 has been complaining for weeks about a time zone conversion test. I have now downloaded a version, and changed the code so that it will work in 8.1 as well as prior releases. (It is still likely that the existing code should work in 8.1, but I haven't yet figured out how to file a bug report.) In the course of testing, 3 additional 8.1 problems were reported (all along the lines of "can't pass null to strpos"), and are fixed with null coercion.
- Two Calculation tests failed because of large results on 32-bit system. These are corrected by allowing the functions involved to return float|int rather than int. I suspect that there are other functions with this problem, and will investigate as a follow-up activity.
- See issue #2090. I believe that changes between 17.1 and master will merely cause the problematic spreadsheet to fail in a different way. I believe that enclosing in quotes some variables passed to Document/Properties by Reader/Xlsx will eliminate the problem, but, in the absence of an example file, cannot say for sure.
- Properties tests are now separated out from Reader/XlsxTest and Reader/OdsTest, and now test both Read and Write (via reload).
<!-- end of other changes list -->
Miscellaneous Notes:
- There remains no support for Custom Properties in Xls Reader or Writer.
- We now have default timezones for all of PHP itself, Shared/Date, and Shared/Timezone. That is least one too many. I was unable to disentangle the latter two for this change, but will look into deprecating one or the other in future.
* Phpstan
6 baseline deletions, 2 docblock changes
* Scrutinizer's Turn
3 minor errors that hadn't blocked the request.
Both methods used to optionally return null if passed a
second argument. This second argument was removed entirely and the
method always returns a RowDimension or ColumnDimension respectively
(possibly creating it if needed).
This make the API more predictable and easier to do static analysis
with tools such as PHPStan.
If you relied on that second parameter, you should instead use the
`Worksheet::getRowDimensions()` or `Worksheet::getColumnDimensions()` and
check for existence yourself before calling the getters.
* Stacked Alignment - Use Class Constant Rather than Literal
PR #1580 defined constants for "stacked" alignment in cells.
Using those constants outside of Style/Alignment was beyond the
scope of the original PR, but I said I would get to it.
This PR replaces all uses of literal -165, and appropriate uses of
literal 255, with the named constants, and adds tests to make sure
that the changed code is covered in the test suite.
Sheet View Settings Block should be 8-14 bytes long in BIFF8 Excel 97 according
to the open office file format documentation. However access to byte 10 and 12 is
not possible when record data is malformed, so getUInt2d throws notice.
If BIFF5 excel 95 file doesn't have codepage record, the default codepage CP1252 is used and can't be change.
That causes to problems with decoding cyrillic text.
This problem is the same as #1238, which was resolved by #1239.
For that issue, the fix was to check in one place whether
$this->mapCellXfIndex[$xfIndex] was set before using it.
The sample spreadsheet supplied as a description for this
problem had exactly the same problem in 2 other places in the code.
In addition, there were 7 other places in the code where that
particular item was used unchecked. This fix corrects all 9 locations.
The spreadsheet supplied with the problem is used as the basis
for some new tests, which particularly test column dimensions
and styles, the problems involved in this case.
When extracting sheet title from string reference (like `"Work!sheet1!A1"`), PHP function `explode()` divide this string into three parts: `['Work', 'sheet1', 'A1']`. And then these wrong values are used in formulas, ranges, etc.
This change fix that problem by using special function `Worksheet::extractSheetTitle()`. This function also has been changed to make sure that worksheet title can contain "!" character. So, that function search last position of "!" in reference string and divide it to 2 parts correctly: `['Work!sheet1', 'A1']`.
Fixes#325Fixes#662
This a bugfix for php 7.3 related errors where using continue inside a switch raises a PHP warning. Either use continue 2 or break. Previously continue behaves like break but the intended usage is to continue the for loop instead.
Column indexes are always based on 1 everywhere in PhpSpreadsheet.
This is consistent with rows starting at 1, as well as Excel
function `COLUMN()`. It should also make it easier to reason about
columns and rows and remove any doubts whether a specific method is
expecting 0 based or 1 based indexes.
Fixes#273
Fixes https://github.com/PHPOffice/PHPExcel/issues/307
Fixes https://github.com/PHPOffice/PHPExcel/issues/476
oleibman
Mark Baker
Adrien Crivelli
oleibman
Georgiy
Gianluca Giovinazzo
Jan-Simon Winkelmann
Pavel Alazankin
Dennis Holst
Diego Souza
Jon Dufresne
Timur
Roland Eigelsreiter
Paul Klimov
Alessandro Lai
Adrien Cohen