Improving Coverage for Excel2003 XML Reader #1557
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resync with base project
Sync with Base
Sync Up With Base
Reader/Xml is now 100% covered. File templates/Excel2003XMLTest.xml, used in some tests, is *not* readable by a current version of Excel. I have substituted a new file excel2003.xml to be used in its place. I have not deleted the original in case someone in future (possibly me) wants to see what it needs to make it usable. There are minimal code changes. - Unused protected functions pixel2WidthUnits and widthUnits2Pixel are deleted. - One regex looking to convert hex characters is changed from a-z to a-f, and made case insensitive. - No calculation performed for "error" cell (previously calculation was attempted and threw exception). - Empty relative row/cell is now handled correctly. - Style applied to empty cell when appropriate. - Support added for textRotation. - Support added for border styles. - Support added for diagonal borders. - Support added for superscript and subscript. - Support added for fill patterns. In theory, encodings other than UTF-8 were supported. In fact, I was unable to get SecurityScanner to pass *any* xml which is not UTF-8. Eliminating the assumption that strings might not be UTF-8 allowed much of the code to be greatly simplified. After that, I added some code that would permit the use of some ASCII-compatible encodings (there is a test of ISO-8859-1). It would be more difficult to handle other encodings (such as UTF-16). I am not convinced that even the ISO-8859 effort is worth it, but am willing to investigate either expanding or eliminating non-UTF8 support. I added a number of tests, creating an Xml directory, and moving XmlTest to that directory. Pull Request had problems reading old invalid sample in the code coverage phase, not in any of the other test phases, and not in the code coverage phase on my local machine. As it turns out, aside from being invalid, the sample is much larger than any of the other samples. Tests have been adjusted accordingly.
Should eliminate need to avoid test during xml coverage.
MarkBaker
reviewed
Jul 15, 2020
|
It's good to have better testing to ensure that future changes don't break anything, and I appreciate all your efforts for helping us improve the PHPSpreadsheet library |
Per suggestion from Mark Baker.
The introduction of AddressHelper introduced a conflict which needed to be resolved. I wanted to test it locally before resolving. This required me to add (unchanged) AddressHelper to my local copy. I hope this is an okay manner of resolving the conflict.
XmlOddTest works just fine on my local machine, but Travis failed it. Even worse, the lines which Travis flags don't even make any sense (one was the empty line between two methods!). This test is not essential to the rest of the change. I am removing it from the package, and will attempt to re-add it when I have a chance to sync up my fork with the main project.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
Removing stalebot tag. |
ggiovinazzo
pushed a commit
to MediadataTV/PhpSpreadsheet
that referenced
this pull request
Dec 14, 2020
* Improving Coverage for Excel2003 XML Reader Reader/Xml is now 100% covered. File templates/Excel2003XMLTest.xml, used in some tests, is *not* readable by a current version of Excel. I have substituted a new file excel2003.xml to be used in its place. I have not deleted the original in case someone in future (possibly me) wants to see what it needs to make it usable. There are minimal code changes. - Unused protected functions pixel2WidthUnits and widthUnits2Pixel are deleted. - One regex looking to convert hex characters is changed from a-z to a-f, and made case insensitive. - No calculation performed for "error" cell (previously calculation was attempted and threw exception). - Empty relative row/cell is now handled correctly. - Style applied to empty cell when appropriate. - Support added for textRotation. - Support added for border styles. - Support added for diagonal borders. - Support added for superscript and subscript. - Support added for fill patterns. In theory, encodings other than UTF-8 were supported. In fact, I was unable to get SecurityScanner to pass *any* xml which is not UTF-8. Eliminating the assumption that strings might not be UTF-8 allowed much of the code to be greatly simplified. After that, I added some code that would permit the use of some ASCII-compatible encodings (there is a test of ISO-8859-1). It would be more difficult to handle other encodings (such as UTF-16). I am not convinced that even the ISO-8859 effort is worth it, but am willing to investigate either expanding or eliminating non-UTF8 support. I added a number of tests, creating an Xml directory, and moving XmlTest to that directory. Pull Request had problems reading old invalid sample in the code coverage phase, not in any of the other test phases, and not in the code coverage phase on my local machine. As it turns out, aside from being invalid, the sample is much larger than any of the other samples. Tests have been adjusted accordingly. * Smaller Test File Should eliminate need to avoid test during xml coverage. * Break Up Style Test into Multiple Tests Per suggestion from Mark Baker. * Integrate AddressHelper Change The introduction of AddressHelper introduced a conflict which needed to be resolved. I wanted to test it locally before resolving. This required me to add (unchanged) AddressHelper to my local copy. I hope this is an okay manner of resolving the conflict. * Weird Travis Error XmlOddTest works just fine on my local machine, but Travis failed it. Even worse, the lines which Travis flags don't even make any sense (one was the empty line between two methods!). This test is not essential to the rest of the change. I am removing it from the package, and will attempt to re-add it when I have a chance to sync up my fork with the main project.
jmourell
added a commit
to visualalpha/PhpSpreadsheet
that referenced
this pull request
Apr 6, 2021
* xdebug isn't built into the nightly PHP, so we'll get an error if we try to remove it; nightly should also be allow failures untilany identified issues are resolved
* <>/!=
* Update to versions of complex and matrix libraries
* Set formula attribute in the Xlsx reader
* Calculation/DateTime Failure With PHP8 (#1661)
The following code generates an error with PHP8:
if ($t1[1] > 29) {
$t1[1] += 1900;
Under the "right" conditions, PHP8 evaluates the condition as true
when PHP8 is a non-numeric string and then generates an error trying
to perform += on that non-numeric string. Adding a numeric test
eliminates the problem. All unit tests involving this code now
succeed with both PHP7 and PHP8.
* Update to versions of complex and matrix libraries
* Reader/Gnumeric Failure with PHP8 (#1662)
* Reader/Gnumeric Failure with PHP8
An explicit cast from SimpleXML to int is now needed with PHP8,
where it was performed implicitly with PHP7.
Unit tests run correctly for both PHP7 and PHP8 on corrected code.
* ReverseSort bug, exposed but not caused by PHP8 (#1660)
Some tests of ReferenceHelper functions columnReverseSort and
cellReverseSort which passed with PHP7 fail with PHP8.
Both functions use the following construction:
return 1 - strcasecmp(whatever);
The "1" seems very mysterious. I believe that the correct code should be:
return -strcasecmp(whatever);
It appears in particular that PHP7 strcasecmp was never returning a
value of 1 for the tests in question, but PHP8 strcasecmp does so.
With the corrected code, the tests pass in both PHP7 and PHP8.
* Drop $this->spreadSheet null check from Xlsx Writer (#1646)
$this->spreadSheet cannot be null
* Update index.md (#1620)
* Bug setting Superscript/Subscript to false (#1567)
If font style Superscript is set to true, Subscript is set to false.
Likewise, setting Subscript to true sets Superscript to false.
Both of these are working as they should. However,
setting Superscript to false causes Subscript to be set to true,
and setting Subscript to false causes Superscript to be set to true.
I believe that is an error in both cases. This change fixes it.
There seem to be no existing tests for Font styles.
I added the tests necessary to validate this change.
I will put adding more on my to-do list.
* Update CHANGELOG
* Sync README
* Separate compoer.json for PHP8 to enforce phpunit ^9.3
* We don't want the composer lock file for PHP8
* Work with existing composer file; but force a fresh install to require new dependency versions
* Allow failures on nightly
* Allow failures on nightly fails if we remove it from the grid
* fixed php8 deprecation warning for libxml_disable_entity_loader() (#1625)
* fixed php8 deprecation warning for libxml_disable_entity_loader()
* Use ZipArchive methods as zip_* functions are deprecated in PHP8
* Remove tests that include tcPDF and DomPDF libraries when running against PHP8, because neither library is yet PHP8-ready
* Let's try to figure out why we're having an issue with a temp csv file
* Need to add `application/csv` to the list of valid CSV mime types
* Locale-specific float to string is changed in PHP as per https://wiki.php.net/rfc/locale_independent_float_to_string
* Codestyle clean-ups
* Switch from using poser badges to shields.io because poser wasn't picking up the correct license
* Update composer suggestions
* Introduce GitHub Actions
The plan is to keep Travis for a short while, until we are confident that
GitHub Actions work well enough for us. And after that we can remove Travis
entirely.
There is a bunch of duplicated things but it allows us to maximize
parallelismt to have results as soon as possible.
API documentation generation is still missing.
* Annotate problems in code
* Publish API docs via GitHub Actions
* Update composer suggestions
* Test PHP 8 on GitHub Actions
* Improving Coverage for Excel2003 XML Reader (#1557)
* Improving Coverage for Excel2003 XML Reader
Reader/Xml is now 100% covered.
File templates/Excel2003XMLTest.xml, used in some tests, is *not*
readable by a current version of Excel. I have substituted a new file
excel2003.xml to be used in its place. I have not deleted the original
in case someone in future (possibly me) wants to see what it needs to
make it usable.
There are minimal code changes.
- Unused protected functions pixel2WidthUnits and widthUnits2Pixel
are deleted.
- One regex looking to convert hex characters is changed from a-z to a-f,
and made case insensitive.
- No calculation performed for "error" cell (previously calculation
was attempted and threw exception).
- Empty relative row/cell is now handled correctly.
- Style applied to empty cell when appropriate.
- Support added for textRotation.
- Support added for border styles.
- Support added for diagonal borders.
- Support added for superscript and subscript.
- Support added for fill patterns.
In theory, encodings other than UTF-8 were supported.
In fact, I was unable to get SecurityScanner to pass *any* xml which is
not UTF-8. Eliminating the assumption that strings might not be UTF-8
allowed much of the code to be greatly simplified.
After that, I added some code that would permit the use of
some ASCII-compatible encodings (there is a test of ISO-8859-1).
It would be more difficult to handle other encodings (such as UTF-16).
I am not convinced that even the ISO-8859 effort is worth it,
but am willing to investigate either expanding or eliminating
non-UTF8 support.
I added a number of tests, creating an Xml directory, and moving
XmlTest to that directory.
Pull Request had problems reading old invalid sample in the code
coverage phase, not in any of the other test phases, and not in
the code coverage phase on my local machine.
As it turns out, aside from being invalid, the sample
is much larger than any of the other samples. Tests have been
adjusted accordingly.
* Smaller Test File
Should eliminate need to avoid test during xml coverage.
* Break Up Style Test into Multiple Tests
Per suggestion from Mark Baker.
* Integrate AddressHelper Change
The introduction of AddressHelper introduced a conflict which needed to
be resolved. I wanted to test it locally before resolving. This required
me to add (unchanged) AddressHelper to my local copy. I hope this is
an okay manner of resolving the conflict.
* Weird Travis Error
XmlOddTest works just fine on my local machine, but Travis failed it.
Even worse, the lines which Travis flags don't even make any sense
(one was the empty line between two methods!).
This test is not essential to the rest of the change. I am removing
it from the package, and will attempt to re-add it when I have a chance
to sync up my fork with the main project.
* Update CHANGELOG
* 1.15.0
* Remove coverage from Travis
Coverage data were not correctly uploaded for a long time now, and it
now is uploaded via GitHub Actions
* Prepare for next version
* Ensure that the list of shared formulae is maintained while chunk-reading Xlsx Files (#1680)
* Ensure that the list of shared formulae is maintained while chunk-reading Xlsx files
* Prevent notice during accessing "cached magnification factor" offset
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.
* Refresh lock files
* Add exportArray Method for Styles (#1580)
Issue #580 has gone stale since I started work on this.
Nevertheless, this implements an exportArray function as an
exact counterpart of applyFromArry.
I chose the name exportArray to avoid confusion with the existing
method getStyleArray, which does something completely different.
This change also increases coverage for all the Style classes to 100%,
with the exception of Style.php itself. There were several (unchanged)
places in Style.php where I did not have sufficient understanding of
what was supposed to be happening, so could not create tests.
All properties used by applyFromArray are exported by this method.
Note that conditional styles are not covered; this is consistent
with the fact that they are not covered by applyFromArray.
The method is implemented as a final public function in Style/Supervisor,
which calls abstract protected function exportArray1, which is implemented
in each of the subclasses, and which calls final protected
function exportArray2 in Style/Supervisor.
So exportArray is usable for any of the subclasses as well.
The new method is added to the documentation.
The existing documentation for applyFromArray was alphabetized to make
it easier to follow.
One property (Style quotePrefix) was added to the documentation.
Some Borders pseudo-properties (vertical, horizontal, and outline) were
documented as usable by applyFromArray,
but aren't actually supported - they were removed.
The documentation of the properties seemed to use setProperty and
getProperty fairly randomly - it now uses setProperty exclusively.
New constants were added for the textRotation "angles" used to create a
"stacked" cell. I felt that changing the readers and writers to use
these constants was beyond the scope of this change, but it is
on my to-do list.
* Updating a misspelling of a function name. (#1695)
This will update the function name DCOUNTA from the misspelling of DCOUNT.
* Make DefinedNames Samples Consistent With Other Samples (#1707)
All other Samples write to temporary directory. DefinedNames samples
write to main directory, which (a) means they aren't stored with others,
and (b) they aren't ignored by git so look like changed files.
The tests are also simplified by requiring Header rather than Bootstrap,
making use of Helper.
* Resolve XSS Vulnerability in the HTML Writer (#1719)
Resolve XSS Vulnerability in the HTML Writer
* Drop Travis
* Automatic GitHub releases from git tags
* Improve Coverage in src/PhpSpreadsheet
There are no changes to code. Additional tests are added,
so that the following 6 items now have 100% test coverage:
- Comment
- DefinedName
- DocumentGenerator
- IOFactory
- NamedFormula
- NamedRange
* Changes for Scrutinizer
Two changes to fix minor problems reported by Scrutinizer.
* Spelling: Tou -> You
* Fix for 1735 (Incorrect activeSheetIndex after RemoveSheetByIndex) (#1743)
This is a fix for issue #1735.
It adds tests for this situation, and similar situations involving
adding new sheets and accessing existing ones.
Coverage for Spreadsheet.php increases from 69% to 75% as a result.
* Update change log
* Fix for 3 Issues Involving ReadXlsx and NamedRange (#1742)
* Fix for 3 Issues Involving ReadXlsx and NamedRange
Issues #1686 and #1723, which provide sample spreadsheets, are probably
solved by this ticket. Issue #1730 is also probably solved, but I have
no way to verify.
There are two problems with how PhpSpreadsheet is handling things now.
Although the first problem is much less severe, and isn't really a factor
in the issues named above, it is helpful to get it out of the way first.
If you define a named range in Excel, and then delete the sheet where
the range exists, Excel saves the range as #REF!. If there is a cell which
references the range, it will similarly have the value #REF! when you open
the Excel file.
Currently, PhpSpreadsheet discards the #REF! definition, so a cell which
references the range will appear as #NAME? rather than #REF!.
This PR changes the behavior so that PhpSpreadsheet retains the #REF!
definition, and cells which reference it will appear as #REF!.
The second problem is the more severe, and is, I believe, responsible
for the 3 issues identified above.
If you define a named range and the sheet on which the range is defined
does not exist at the time, Excel will save the range as something like:
'[1]Unknown Sheet'!$A$1
If a cell references such a range, Excel will again display #REF!.
PhpSpreadsheet currently throws an Exception when it encounters
such a definition while reading the file. This PR changes
the behavior so that PhpSpreadsheet saves the definition as #REF!,
and cells which reference it will behave similarly.
For the record, I will note that Excel does not magically recalculate when a
missing sheet is subsequently added, despite the fact that the reference
might now become resolvable. PhpSpreadsheet behaves likewise.
* Remove Dead Code in Test
Identified it after push but before merge.
* Update change log
* Apply Column and Row Styles to Existing Cells (#1721)
* Apply Column and Row Styles to Existing Cells
This is a fix for issue #1712.
When a style is applied to an entire row or column, it is currently
only effective for cells which don't already contain a value.
The code needs to iterate through existing cells in the row/column
in order to apply the style to them.
This could be considered a breaking change, however, I believe that
the change makes things operate as users would expect, and that the
existing implementation is incomplete.
The change also removes protected element conditionalStyles from
the Style class. That element is an unused remnant, and can no longer be
set or retrieved - methods getConditionalStyles and setConditionalStyles
actually act on an element in the Worksheet class.
Finally, additional tests are added so that Style, and in fact the
entire Style directory, now has 100% test coverage.
* Scrutinizer Changes
Scrutinizer flagged 6 statements. 5 can be easily corrected.
One is absolutely wrong (it thinks iterating through cells in column
can return null). Let's see if we can satisfy it.
* Remove Exception For CellIterator on Empty Row/Column
For my first attempt at this change, which corrects a bug by updating styles
for non-empty cells when a style is set on a row or column, I wished to make things
more efficient by using setIterateOnlyExistingCells, something which the
existing documentation recommends. This caused an exception to be generated
when the row or column is empty. So I removed that part of the change while I
researched what was going on.
I have completed that research. The existing code does throw an exception
when the row/column is empty and iterateOnlyExistingCells is true. However,
that does not seem like a reasonable action. This situation is analagous to
iterating over an empty array, and that action is legal and does not throw.
The same should apply here. There were no tests for this situation,
and now there are.
I have added additional tests, and coverage for all of RowCellIterator,
ColumnCellIterator, and CellIterator are all now 100%. Some of my new tests
were added in new members, because the existing tests all relied on mocking,
which was not the best choice for the new tests. One of the existing tests
for RowCellIteratorTest (testSeekOutOfRange) was wrong; it issued the expected
exception, but for the wrong reason. I have added an additional test to
ensure that it fails "correctly".
The existing documentation says that the default value for
IterateOnlyExistingCells is true. In fact, the default value is false.
I have corrected the documentation.
* More Scrutinizer
I believe its analysis is incorrect, but this should silence it.
* DocBlock Correction
ColumnCellIterator DocBlock for current indicated it could return null
or Cell, but it can really return only Cell. This had caused Scrutinizer
to complain earlier.
* PHP8 Environment Appears to be Fixed
Cosmetic change to Doc member. I suspect there is a way to rerun all
the tests without another push, but I have been unable to figure out how.
* Update change log
* TextData Coverage and Minor Bug Fixes (#1744)
This had been intended to get 100% coverage for TextData functions, and it does that.
However, some minor bugs requiring source changes arose during testing.
- the Excel CHAR function restricts its argument to 1-255. PhpSpreadsheet CHARACTER
had been allowing 0+. Also, there is no need to test if iconv exists,
since it is part of Composer requirements.
- The DOLLAR function had been returning NUM for invalid arguments. Excel returns VALUE.
Also, negative amounts were not being handled correctly.
- The FIXEDFORMAT function had been returning NUM for invalid arguments. Excel FIXED returns VALUE.
* Replace anti-xss with html purifier (#1751)
* Replace voku/anti-xss with ezyang/htmlpurifier. Despite anti-xss being a smaller footprint dependency, an a better license fit with our MIT license, there are issues with it's automatic it sanitisation of global variables causing side effects
* Additional unit tests for xss in html writer cell comments
* Fix bug #1626 where values of 0 were "rounded" up/down as if they were not 0 (#1627)
* Fix bug where values of 0 were "rounded" up/down as if they were not 0
* Update change log
* Fix for #1612 - SLK Long File Name (#1706)
Issue has been marked stale, but ...
Sylk read sets worksheet title to filename (minus .slk).
If that is >31 characters, PhpSpreadsheet throws Exception.
This change truncates sheet title, as Excel does, to 31 characters.
* Update change log
* worksheet: fix if cellValue does not exist (#1727)
The condition is FALSE if the cell does not exist in the flipped table,
but anyway, it is sent in to a method requiring 'string' type, causing
it to fail.
* fixes #1655 issue (#1656)
Resolve problem with incorrectly defined hyperlinks
* Add 'ps' suffix to printer settings resources IDs (#1690)
* Add 'ps' suffix to printer settings resources IDs
* Update change log
* Fix pixelsToPoints conversion (for HTML col width) (#1733)
* DocBlock Change in Styles/Conditional (#1697)
Scrutinizer reported a minor error in a test involving a module
which I was not changing.
Styles/Conditional function setConditions can take a scalar or an
array as a parameter, but DocBlock says it only expects array.
I did not wish to add the extra module to my PR, but made a note to
self to fix that after PR was installed.
That has now happened, and it makes for a good case for me
to see all the PHP8/Composer2/etc. changes that have happened recently.
* Merge pull request #1698
* Merge pull request #4 from PHPOffice/master
* Restore Omitted Read XML Test
* Fix for bug #1592 (UPDATED) (#1623)
* Fix for Xls when BIFF8 SST (FCh) has bad Shared string length
* Update change log
* Add nightly PHP 8.1 dev to github actions (#1763)
* Fix compatibility with ext-gd on php 8 (#1762)
* CSV - Guess Encoding, Handle Null-string Escape (#1717)
* CSV - Guess Encoding, Handle Null-string Escape
This is in response to issue #1647 (detect CSV character encoding).
First, my tests with mb_detect_encoding indicate that it doesn't work
well enough; regardless, users can always do that on their own
if they deem it useful.
Rolling my own is also troublesome, but I can at least:
a. Check for BOM (UTF-8, UTF-16BE, UTF-16LE, UTF-32BE, UTF-32LE).
b. Do some heuristic tests for each of the above encodings.
c. Fallback to a user-specified encoding (default CP1252)
if a and b don't yield result.
I think this is probably useful enough to include, and relatively
easy to expand if other potential encodings should be considered.
Starting with PHP7.4, fgetcsv allows specification of null string as
escape character in fgetcsv. This is a much better choice than the PHP
(and PhpSpreadsheet) default of backslash in that it handles the file
in the same manner as Excel does. There is one statement in Reader/CSV
which would be adversely affected if the caller so specified (building
a regular expression under the assumption that escape character is
a single character). Fix that statement appropriately and add tests.
* Update changelog
* Update Units of Measure supported by the CONVERT() function (#1768)
Now supports all current UoM in all categories, with both 1- and 2-character multiplier prefixes, and binary multiplier prefixes, including the new Temperature scales
* Changelog for 1.16.0 release
* Fix date tests withut specified year for current year 2021 (#1774)
* Mrand of zero to any multiple should return 0 (#1773)
* Problems Using Builtin PHP Functions Directly As Excel Functions (#1799)
* Problems Using Builtin PHP Functions Directly As Excel Functions
This fixes issue #1789.
As originally reported, stricter typing was causing PHP8 to throw
an exception when a non-numeric value was passed to the Round function.
Previous releases of PHP did not see this problem, however, on further
analysis, they were also incorrect in returning 0 as the result in the
erroneous situation, when they should have been returning a VALUE error.
Yet more analysis showed that other functions would also have problems,
and, in addition, might not handle invalid input (e.g. a negative length
passed to REPT) or output (e.g. NAN in the case of ACOS(2)) correctly.
The following MathTrig functions are affected:
ABS, ACOS, ACOSH, ASIN, ASINH, ATAN, ATANH,
COS, COSH, DEGREES (rad2deg), EXP, LN (log), LOG10,
RADIANS (deg2rad), REPT (str_repeat), SIN, SINH, SQRT, TAN, TANH.
One TextData function (REPT) is also affected.
This change lets PhpSpreadsheet validate the input for each of these
functions before passing control to the builtin, and handle the output
afterwards.
There were no explicit tests for any of these functions, a fact made
easy to ignore by the fact that PhpSpreadsheet delegated the heavy
lifting to PHP itself for these cases. A full suite of tests is
now added for each of the affected functions.
* Scrutinizer Recommendations
Only in 3 modules which are part of this PR.
* Improved Handling of Tan(PI/2)
Return DIV0 error for TAN when COS is very small.
* Additional Trig Tests
Results which should be infinity, i.e. DIV/0 error.
* Delete Temporary Files In XssVulnerabilityTest (#1800)
* Delete Temporary Files In XssVulnerabilityTest
They need not exist after the test. Some of them are placed in
current directory, which means Git thinks they are needed.
* Update Changelog for switch from built-in PHP functions to Excel class methods (to handle typechecking for stricter PHP8 typing)
* Fix For #1772 Null Exception on ODS Read (#1776)
Fix for #1772.
Header and Footer Properties may be omitted in Page Setting Style Set.
Code changed to allow for this possibility, and tests added.
* Additional method call/return typing
* Update Changelog
* PHPCS Resolutions
* Fix Xlsx reader overriding manually set number format with builtin number format (#1805)
* Update Changelog
* Named formula fix for empty formula value
* Do not swallow previous exception (#1778)
The previous exception will be included when loading the content as DOM Document fails. This makes debugging the reason behind the failure much easier.
* Support DataBar of conditional formatting rule (#1754)
Implemented the databar of Conditional Type for XLSX Files.
- DataBar can be read, written, and added for basic use.
- Supports reading, writing and adding using "extLst".
About "extLst"
- https://docs.microsoft.com/en-us/openspecs/office_standards/ms-xlsx/07d607af-5618-4ca2-b683-6a78dc0d9627
The following setting items on the Excel setting screen can be read, written, and added.
- (minimum, maximum)type: Automatic, LowestValue, Number, Percent, Formula, Percentile
- Direction: context, leftToRight, rightToLeft (show data bar only)
- Fills Solid, Gradient
- FillColor: PositiveValues, NegativeValues
- Borders: Solid, None
- BorderColor: PositiveValues, NegativeValues
- Axis position: Automatic, Midpoint, None
- Axis color
* formatAsDate strip language metadata (#1618)
* Revert "Fix cant get right format chinese date format error"
This reverts commit 8c58385d6c103d23f6a5b3d1899a5e7cc8f66e92.
* formatAsDate strip language metadata (fixes #1616)
Co-authored-by: Mark Baker <[email protected]>
* Additional method call/return typing
* Composer fixes
* PHPCS Fixes
* Additional unit tests for statistical functions, with a fix to ordering for RANK() (#1813)
* Additional unit tests for statistical functions, with a fix to ordering for RANK()
* Unhappy path unit tests (#1814)
* Unhappy path unit tests
* Fix unhappy error for BETADIST and BETAINV min/max range
* Additional unit tests for previously untested financial functions (#1815)
* Additional unit tests for previously untested financial functions, and some additions to follow untested paths
* Start splitting Financial function tests out from the large FinancialTests class into individual test classes for each function
* Extract remaining Excel function unit tests into separate test classes for each function (#1817)
* Extract remaining Financial function unit tests into separate test classes for each function
This makes it easier to manage unit tests if they are individual files rather than all in a single file
It also provides a stepping stone toward making it easier to test Excel functions when Excel errors no longer return a string, but an actual Excel exception that can be handled more cleanly
* Additional statistical unit tests (#1818)
* Inherited Scrutinizer Recommendations - 1 of 3 (#1806)
* Make DefinedNames Samples Consistent With Other Samples (#1707)
All other Samples write to temporary directory. DefinedNames samples
write to main directory, which (a) means they aren't stored with others,
and (b) they aren't ignored by git so look like changed files.
The tests are also simplified by requiring Header rather than Bootstrap,
making use of Helper.
* Resolve XSS Vulnerability in the HTML Writer (#1719)
Resolve XSS Vulnerability in the HTML Writer
* Drop Travis
* Automatic GitHub releases from git tags
* Improve Coverage in src/PhpSpreadsheet
There are no changes to code. Additional tests are added,
so that the following 6 items now have 100% test coverage:
- Comment
- DefinedName
- DocumentGenerator
- IOFactory
- NamedFormula
- NamedRange
* Changes for Scrutinizer
Two changes to fix minor problems reported by Scrutinizer.
* Spelling: Tou -> You
* Fix for 1735 (Incorrect activeSheetIndex after RemoveSheetByIndex) (#1743)
This is a fix for issue #1735.
It adds tests for this situation, and similar situations involving
adding new sheets and accessing existing ones.
Coverage for Spreadsheet.php increases from 69% to 75% as a result.
* Update change log
* Fix for 3 Issues Involving ReadXlsx and NamedRange (#1742)
* Fix for 3 Issues Involving ReadXlsx and NamedRange
Issues #1686 and #1723, which provide sample spreadsheets, are probably
solved by this ticket. Issue #1730 is also probably solved, but I have
no way to verify.
There are two problems with how PhpSpreadsheet is handling things now.
Although the first problem is much less severe, and isn't really a factor
in the issues named above, it is helpful to get it out of the way first.
If you define a named range in Excel, and then delete the sheet where
the range exists, Excel saves the range as #REF!. If there is a cell which
references the range, it will similarly have the value #REF! when you open
the Excel file.
Currently, PhpSpreadsheet discards the #REF! definition, so a cell which
references the range will appear as #NAME? rather than #REF!.
This PR changes the behavior so that PhpSpreadsheet retains the #REF!
definition, and cells which reference it will appear as #REF!.
The second problem is the more severe, and is, I believe, responsible
for the 3 issues identified above.
If you define a named range and the sheet on which the range is defined
does not exist at the time, Excel will save the range as something like:
'[1]Unknown Sheet'!$A$1
If a cell references such a range, Excel will again display #REF!.
PhpSpreadsheet currently throws an Exception when it encounters
such a definition while reading the file. This PR changes
the behavior so that PhpSpreadsheet saves the definition as #REF!,
and cells which reference it will behave similarly.
For the record, I will note that Excel does not magically recalculate when a
missing sheet is subsequently added, despite the fact that the reference
might now become resolvable. PhpSpreadsheet behaves likewise.
* Remove Dead Code in Test
Identified it after push but before merge.
* Update change log
* Apply Column and Row Styles to Existing Cells (#1721)
* Apply Column and Row Styles to Existing Cells
This is a fix for issue #1712.
When a style is applied to an entire row or column, it is currently
only effective for cells which don't already contain a value.
The code needs to iterate through existing cells in the row/column
in order to apply the style to them.
This could be considered a breaking change, however, I believe that
the change makes things operate as users would expect, and that the
existing implementation is incomplete.
The change also removes protected element conditionalStyles from
the Style class. That element is an unused remnant, and can no longer be
set or retrieved - methods getConditionalStyles and setConditionalStyles
actually act on an element in the Worksheet class.
Finally, additional tests are added so that Style, and in fact the
entire Style directory, now has 100% test coverage.
* Scrutinizer Changes
Scrutinizer flagged 6 statements. 5 can be easily corrected.
One is absolutely wrong (it thinks iterating through cells in column
can return null). Let's see if we can satisfy it.
* Remove Exception For CellIterator on Empty Row/Column
For my first attempt at this change, which corrects a bug by updating styles
for non-empty cells when a style is set on a row or column, I wished to make things
more efficient by using setIterateOnlyExistingCells, something which the
existing documentation recommends. This caused an exception to be generated
when the row or column is empty. So I removed that part of the change while I
researched what was going on.
I have completed that research. The existing code does throw an exception
when the row/column is empty and iterateOnlyExistingCells is true. However,
that does not seem like a reasonable action. This situation is analagous to
iterating over an empty array, and that action is legal and does not throw.
The same should apply here. There were no tests for this situation,
and now there are.
I have added additional tests, and coverage for all of RowCellIterator,
ColumnCellIterator, and CellIterator are all now 100%. Some of my new tests
were added in new members, because the existing tests all relied on mocking,
which was not the best choice for the new tests. One of the existing tests
for RowCellIteratorTest (testSeekOutOfRange) was wrong; it issued the expected
exception, but for the wrong reason. I have added an additional test to
ensure that it fails "correctly".
The existing documentation says that the default value for
IterateOnlyExistingCells is true. In fact, the default value is false.
I have corrected the documentation.
* More Scrutinizer
I believe its analysis is incorrect, but this should silence it.
* DocBlock Correction
ColumnCellIterator DocBlock for current indicated it could return null
or Cell, but it can really return only Cell. This had caused Scrutinizer
to complain earlier.
* PHP8 Environment Appears to be Fixed
Cosmetic change to Doc member. I suspect there is a way to rerun all
the tests without another push, but I have been unable to figure out how.
* Update change log
* TextData Coverage and Minor Bug Fixes (#1744)
This had been intended to get 100% coverage for TextData functions, and it does that.
However, some minor bugs requiring source changes arose during testing.
- the Excel CHAR function restricts its argument to 1-255. PhpSpreadsheet CHARACTER
had been allowing 0+. Also, there is no need to test if iconv exists,
since it is part of Composer requirements.
- The DOLLAR function had been returning NUM for invalid arguments. Excel returns VALUE.
Also, negative amounts were not being handled correctly.
- The FIXEDFORMAT function had been returning NUM for invalid arguments. Excel FIXED returns VALUE.
* Replace anti-xss with html purifier (#1751)
* Replace voku/anti-xss with ezyang/htmlpurifier. Despite anti-xss being a smaller footprint dependency, an a better license fit with our MIT license, there are issues with it's automatic it sanitisation of global variables causing side effects
* Additional unit tests for xss in html writer cell comments
* Fix bug #1626 where values of 0 were "rounded" up/down as if they were not 0 (#1627)
* Fix bug where values of 0 were "rounded" up/down as if they were not 0
* Update change log
* Fix for #1612 - SLK Long File Name (#1706)
Issue has been marked stale, but ...
Sylk read sets worksheet title to filename (minus .slk).
If that is >31 characters, PhpSpreadsheet throws Exception.
This change truncates sheet title, as Excel does, to 31 characters.
* Update change log
* worksheet: fix if cellValue does not exist (#1727)
The condition is FALSE if the cell does not exist in the flipped table,
but anyway, it is sent in to a method requiring 'string' type, causing
it to fail.
* fixes #1655 issue (#1656)
Resolve problem with incorrectly defined hyperlinks
* Add 'ps' suffix to printer settings resources IDs (#1690)
* Add 'ps' suffix to printer settings resources IDs
* Update change log
* Fix pixelsToPoints conversion (for HTML col width) (#1733)
* DocBlock Change in Styles/Conditional (#1697)
Scrutinizer reported a minor error in a test involving a module
which I was not changing.
Styles/Conditional function setConditions can take a scalar or an
array as a parameter, but DocBlock says it only expects array.
I did not wish to add the extra module to my PR, but made a note to
self to fix that after PR was installed.
That has now happened, and it makes for a good case for me
to see all the PHP8/Composer2/etc. changes that have happened recently.
* Merge pull request #1698
* Merge pull request #4 from PHPOffice/master
* Restore Omitted Read XML Test
* Fix for bug #1592 (UPDATED) (#1623)
* Fix for Xls when BIFF8 SST (FCh) has bad Shared string length
* Update change log
* Add nightly PHP 8.1 dev to github actions (#1763)
* Fix compatibility with ext-gd on php 8 (#1762)
* CSV - Guess Encoding, Handle Null-string Escape (#1717)
* CSV - Guess Encoding, Handle Null-string Escape
This is in response to issue #1647 (detect CSV character encoding).
First, my tests with mb_detect_encoding indicate that it doesn't work
well enough; regardless, users can always do that on their own
if they deem it useful.
Rolling my own is also troublesome, but I can at least:
a. Check for BOM (UTF-8, UTF-16BE, UTF-16LE, UTF-32BE, UTF-32LE).
b. Do some heuristic tests for each of the above encodings.
c. Fallback to a user-specified encoding (default CP1252)
if a and b don't yield result.
I think this is probably useful enough to include, and relatively
easy to expand if other potential encodings should be considered.
Starting with PHP7.4, fgetcsv allows specification of null string as
escape character in fgetcsv. This is a much better choice than the PHP
(and PhpSpreadsheet) default of backslash in that it handles the file
in the same manner as Excel does. There is one statement in Reader/CSV
which would be adversely affected if the caller so specified (building
a regular expression under the assumption that escape character is
a single character). Fix that statement appropriately and add tests.
* Update changelog
* Update Units of Measure supported by the CONVERT() function (#1768)
Now supports all current UoM in all categories, with both 1- and 2-character multiplier prefixes, and binary multiplier prefixes, including the new Temperature scales
* Changelog for 1.16.0 release
* Fix date tests withut specified year for current year 2021 (#1774)
* Mrand of zero to any multiple should return 0 (#1773)
* WIP Inherited Scrutinizer Recommendations - 1 of 3
I tried to sync my fork with the main project, as I have done several times before.
However, the GitHub interface to do this had changed, and it appears that I
did not make the optimal selection when I had a choice. Consequently, all
the merges that happened to base between the last time I synchronized and this time
appear to be part of any PR that I push. "Files changed" remains correct for
my new PRs, but there appear to be many more commits involved than is actually the
case. I will, at some point, delete and re-create my fork, and pay much closer
attention in future when I want to sync my fork with the main project.
Because of this set-up, Scrutinizer reports flaws in code that I haven't
actually changed in my PRs #1799 and #1800.
It still passes them, but, as long as I'm aware of the problems,
I may as well attempt to correct them. The following are not part of those PRs:
- 5 problems spread over 4 different members
- 12 problems in Calculation/Engineering
- 15 problems in Reader/XML
I shall attempt to resolve these via 3 separate PRs,
of which this is the first.
* Try Hyperlink Again
Scrutinizer still didn't like it as fixed.
* Another Crack at Hyperlink
This should work.
Co-authored-by: Mark Baker <[email protected]>
Co-authored-by: Adrien Crivelli <[email protected]>
Co-authored-by: Ryan McAllen <[email protected]>
Co-authored-by: Flinsch <[email protected]>
Co-authored-by: Jan Sverre Riksfjord <[email protected]>
Co-authored-by: Max Kalyabin <[email protected]>
Co-authored-by: Sébastien Despont <[email protected]>
Co-authored-by: Guilliam Xavier <[email protected]>
Co-authored-by: Gianluca Giovinazzo <[email protected]>
Co-authored-by: Alexander M. Turek <[email protected]>
Co-authored-by: Martins Sipenko <[email protected]>
* Inherited Scrutinizer Recommendations - 2 of 3 (#1807)
* Make DefinedNames Samples Consistent With Other Samples (#1707)
All other Samples write to temporary directory. DefinedNames samples
write to main directory, which (a) means they aren't stored with others,
and (b) they aren't ignored by git so look like changed files.
The tests are also simplified by requiring Header rather than Bootstrap,
making use of Helper.
* Resolve XSS Vulnerability in the HTML Writer (#1719)
Resolve XSS Vulnerability in the HTML Writer
* Drop Travis
* Automatic GitHub releases from git tags
* Improve Coverage in src/PhpSpreadsheet
There are no changes to code. Additional tests are added,
so that the following 6 items now have 100% test coverage:
- Comment
- DefinedName
- DocumentGenerator
- IOFactory
- NamedFormula
- NamedRange
* Changes for Scrutinizer
Two changes to fix minor problems reported by Scrutinizer.
* Spelling: Tou -> You
* Fix for 1735 (Incorrect activeSheetIndex after RemoveSheetByIndex) (#1743)
This is a fix for issue #1735.
It adds tests for this situation, and similar situations involving
adding new sheets and accessing existing ones.
Coverage for Spreadsheet.php increases from 69% to 75% as a result.
* Update change log
* Fix for 3 Issues Involving ReadXlsx and NamedRange (#1742)
* Fix for 3 Issues Involving ReadXlsx and NamedRange
Issues #1686 and #1723, which provide sample spreadsheets, are probably
solved by this ticket. Issue #1730 is also probably solved, but I have
no way to verify.
There are two problems with how PhpSpreadsheet is handling things now.
Although the first problem is much less severe, and isn't really a factor
in the issues named above, it is helpful to get it out of the way first.
If you define a named range in Excel, and then delete the sheet where
the range exists, Excel saves the range as #REF!. If there is a cell which
references the range, it will similarly have the value #REF! when you open
the Excel file.
Currently, PhpSpreadsheet discards the #REF! definition, so a cell which
references the range will appear as #NAME? rather than #REF!.
This PR changes the behavior so that PhpSpreadsheet retains the #REF!
definition, and cells which reference it will appear as #REF!.
The second problem is the more severe, and is, I believe, responsible
for the 3 issues identified above.
If you define a named range and the sheet on which the range is defined
does not exist at the time, Excel will save the range as something like:
'[1]Unknown Sheet'!$A$1
If a cell references such a range, Excel will again display #REF!.
PhpSpreadsheet currently throws an Exception when it encounters
such a definition while reading the file. This PR changes
the behavior so that PhpSpreadsheet saves the definition as #REF!,
and cells which reference it will behave similarly.
For the record, I will note that Excel does not magically recalculate when a
missing sheet is subsequently added, despite the fact that the reference
might now become resolvable. PhpSpreadsheet behaves likewise.
* Remove Dead Code in Test
Identified it after push but before merge.
* Update change log
* Apply Column and Row Styles to Existing Cells (#1721)
* Apply Column and Row Styles to Existing Cells
This is a fix for issue #1712.
When a style is applied to an entire row or column, it is currently
only effective for cells which don't already contain a value.
The code needs to iterate through existing cells in the row/column
in order to apply the style to them.
This could be considered a breaking change, however, I believe that
the change makes things operate as users would expect, and that the
existing implementation is incomplete.
The change also removes protected element conditionalStyles from
the Style class. That element is an unused remnant, and can no longer be
set or retrieved - methods getConditionalStyles and setConditionalStyles
actually act on an element in the Worksheet class.
Finally, additional tests are added so that Style, and in fact the
entire Style directory, now has 100% test coverage.
* Scrutinizer Changes
Scrutinizer flagged 6 statements. 5 can be easily corrected.
One is absolutely wrong (it thinks iterating through cells in column
can return null). Let's see if we can satisfy it.
* Remove Exception For CellIterator on Empty Row/Column
For my first attempt at this change, which corrects a bug by updating styles
for non-empty cells when a style is set on a row or column, I wished to make things
more efficient by using setIterateOnlyExistingCells, something which the
existing documentation recommends. This caused an exception to be generated
when the row or column is empty. So I removed that part of the change while I
researched what was going on.
I have completed that research. The existing code does throw an exception
when the row/column is empty and iterateOnlyExistingCells is true. However,
that does not seem like a reasonable action. This situation is analagous to
iterating over an empty array, and that action is legal and does not throw.
The same should apply here. There were no tests for this situation,
and now there are.
I have added additional tests, and coverage for all of RowCellIterator,
ColumnCellIterator, and CellIterator are all now 100%. Some of my new tests
were added in new members, because the existing tests all relied on mocking,
which was not the best choice for the new tests. One of the existing tests
for RowCellIteratorTest (testSeekOutOfRange) was wrong; it issued the expected
exception, but for the wrong reason. I have added an additional test to
ensure that it fails "correctly".
The existing documentation says that the default value for
IterateOnlyExistingCells is true. In fact, the default value is false.
I have corrected the documentation.
* More Scrutinizer
I believe its analysis is incorrect, but this should silence it.
* DocBlock Correction
ColumnCellIterator DocBlock for current indicated it could return null
or Cell, but it can really return only Cell. This had caused Scrutinizer
to complain earlier.
* PHP8 Environment Appears to be Fixed
Cosmetic change to Doc member. I suspect there is a way to rerun all
the tests without another push, but I have been unable to figure out how.
* Update change log
* TextData Coverage and Minor Bug Fixes (#1744)
This had been intended to get 100% coverage for TextData functions, and it does that.
However, some minor bugs requiring source changes arose during testing.
- the Excel CHAR function restricts its argument to 1-255. PhpSpreadsheet CHARACTER
had been allowing 0+. Also, there is no need to test if iconv exists,
since it is part of Composer requirements.
- The DOLLAR function had been returning NUM for invalid arguments. Excel returns VALUE.
Also, negative amounts were not being handled correctly.
- The FIXEDFORMAT function had been returning NUM for invalid arguments. Excel FIXED returns VALUE.
* Replace anti-xss with html purifier (#1751)
* Replace voku/anti-xss with ezyang/htmlpurifier. Despite anti-xss being a smaller footprint dependency, an a better license fit with our MIT license, there are issues with it's automatic it sanitisation of global variables causing side effects
* Additional unit tests for xss in html writer cell comments
* Fix bug #1626 where values of 0 were "rounded" up/down as if they were not 0 (#1627)
* Fix bug where values of 0 were "rounded" up/down as if they were not 0
* Update change log
* Fix for #1612 - SLK Long File Name (#1706)
Issue has been marked stale, but ...
Sylk read sets worksheet title to filename (minus .slk).
If that is >31 characters, PhpSpreadsheet throws Exception.
This change truncates sheet title, as Excel does, to 31 characters.
* Update change log
* worksheet: fix if cellValue does not exist (#1727)
The condition is FALSE if the cell does not exist in the flipped table,
but anyway, it is sent in to a method requiring 'string' type, causing
it to fail.
* fixes #1655 issue (#1656)
Resolve problem with incorrectly defined hyperlinks
* Add 'ps' suffix to printer settings resources IDs (#1690)
* Add 'ps' suffix to printer settings resources IDs
* Update change log
* Fix pixelsToPoints conversion (for HTML col width) (#1733)
* DocBlock Change in Styles/Conditional (#1697)
Scrutinizer reported a minor error in a test involving a module
which I was not changing.
Styles/Conditional function setConditions can take a scalar or an
array as a parameter, but DocBlock says it only expects array.
I did not wish to add the extra module to my PR, but made a note to
self to fix that after PR was installed.
That has now happened, and it makes for a good case for me
to see all the PHP8/Composer2/etc. changes that have happened recently.
* Merge pull request #1698
* Merge pull request #4 from PHPOffice/master
* Restore Omitted Read XML Test
* Fix for bug #1592 (UPDATED) (#1623)
* Fix for Xls when BIFF8 SST (FCh) has bad Shared string length
* Update change log
* Add nightly PHP 8.1 dev to github actions (#1763)
* Fix compatibility with ext-gd on php 8 (#1762)
* CSV - Guess Encoding, Handle Null-string Escape (#1717)
* CSV - Guess Encoding, Handle Null-string Escape
This is in response to issue #1647 (detect CSV character encoding).
First, my tests with mb_detect_encoding indicate that it doesn't work
well enough; regardless, users can always do that on their own
if they deem it useful.
Rolling my own is also troublesome, but I can at least:
a. Check for BOM (UTF-8, UTF-16BE, UTF-16LE, UTF-32BE, UTF-32LE).
b. Do some heuristic tests for each of the above encodings.
c. Fallback to a user-specified encoding (default CP1252)
if a and b don't yield result.
I think this is probably useful enough to include, and relatively
easy to expand if other potential encodings should be considered.
Starting with PHP7.4, fgetcsv allows specification of null string as
escape character in fgetcsv. This is a much better choice than the PHP
(and PhpSpreadsheet) default of backslash in that it handles the file
in the same manner as Excel does. There is one statement in Reader/CSV
which would be adversely affected if the caller so specified (building
a regular expression under the assumption that escape character is
a single character). Fix that statement appropriately and add tests.
* Update changelog
* Update Units of Measure supported by the CONVERT() function (#1768)
Now supports all current UoM in all categories, with both 1- and 2-character multiplier prefixes, and binary multiplier prefixes, including the new Temperature scales
* Changelog for 1.16.0 release
* Fix date tests withut specified year for current year 2021 (#1774)
* Mrand of zero to any multiple should return 0 (#1773)
* Inherited Scrutinizer Recommendations - 2 of 3
I tried to sync my fork with the main project, as I have done several times before.
However, the GitHub interface to do this had changed, and it appears that I
did not make the optimal selection when I had a choice. Consequently, all
the merges that happened to base between the last time I synchronized and this time
appear to be part of any PR that I push. "Files changed" remains correct for
my new PRs, but there appear to be many more commits involved than is actually the
case. I will, at some point, delete and re-create my fork, and pay much closer
attention in future when I want to sync my fork with the main project.
Because of this set-up, Scrutinizer reports flaws in code that I haven't
actually changed in my PRs #1799 and #1800.
It still passes them, but, as long as I'm aware of the problems,
I may as well attempt to correct them. The following are not part of those PRs:
- 5 problems spread over 4 different members
- 12 problems in Calculation/Engineering
- 15 problems in Reader/XML
I shall attempt to resolve these via 3 separate PRs,
of which this is the second.
* Fixed Most of the Problems, But Some New Ones Cropped Up
Trying once more to satisfy Scrutinizer.
Co-authored-by: Mark Baker <[email protected]>
Co-authored-by: Adrien Crivelli <[email protected]>
Co-authored-by: Ryan McAllen <[email protected]>
Co-authored-by: Flinsch <[email protected]>
Co-authored-by: Jan Sverre Riksfjord <[email protected]>
Co-authored-by: Max Kalyabin <[email protected]>
Co-authored-by: Sébastien Despont <[email protected]>
Co-authored-by: Guilliam Xavier <[email protected]>
Co-authored-by: Gianluca Giovinazzo <[email protected]>
Co-authored-by: Alexander M. Turek <[email protected]>
Co-authored-by: Martins Sipenko <[email protected]>
* Inherited Scrutinizer Recommendations - 3 of 3 (#1808)
* Make DefinedNames Samples Consistent With Other Samples (#1707)
All other Samples write to temporary directory. DefinedNames samples
write to main directory, which (a) means they aren't stored with others,
and (b) they aren't ignored by git so look like changed files.
The tests are also simplified by requiring Header rather than Bootstrap,
making use of Helper.
* Resolve XSS Vulnerability in the HTML Writer (#1719)
Resolve XSS Vulnerability in the HTML Writer
* Drop Travis
* Automatic GitHub releases from git tags
* Improve Coverage in src/PhpSpreadsheet
There are no changes to code. Additional tests are added,
so that the following 6 items now have 100% test coverage:
- Comment
- DefinedName
- DocumentGenerator
- IOFactory
- NamedFormula
- NamedRange
* Changes for Scrutinizer
Two changes to fix minor problems reported by Scrutinizer.
* Spelling: Tou -> You
* Fix for 1735 (Incorrect activeSheetIndex after RemoveSheetByIndex) (#1743)
This is a fix for issue #1735.
It adds tests for this situation, and similar situations involving
adding new sheets and accessing existing ones.
Coverage for Spreadsheet.php increases from 69% to 75% as a result.
* Update change log
* Fix for 3 Issues Involving ReadXlsx and NamedRange (#1742)
* Fix for 3 Issues Involving ReadXlsx and NamedRange
Issues #1686 and #1723, which provide sample spreadsheets, are probably
solved by this ticket. Issue #1730 is also probably solved, but I have
no way to verify.
There are two problems with how PhpSpreadsheet is handling things now.
Although the first problem is much less severe, and isn't really a factor
in the issues named above, it is helpful to get it out of the way first.
If you define a named range in Excel, and then delete the sheet where
the range exists, Excel saves the range as #REF!. If there is a cell which
references the range, it will similarly have the value #REF! when you open
the Excel file.
Currently, PhpSpreadsheet discards the #REF! definition, so a cell which
references the range will appear as #NAME? rather than #REF!.
This PR changes the behavior so that PhpSpreadsheet retains the #REF!
definition, and cells which reference it will appear as #REF!.
The second problem is the more severe, and is, I believe, responsible
for the 3 issues identified above.
If you define a named range and the sheet on which the range is defined
does not exist at the time, Excel will save the range as something like:
'[1]Unknown Sheet'!$A$1
If a cell references such a range, Excel will again display #REF!.
PhpSpreadsheet currently throws an Exception when it encounters
such a definition while reading the file. This PR changes
the behavior so that PhpSpreadsheet saves the definition as #REF!,
and cells which reference it will behave similarly.
For the record, I will note that Excel does not magically recalculate when a
missing sheet is subsequently added, despite the fact that the reference
might now become resolvable. PhpSpreadsheet behaves likewise.
* Remove Dead Code in Test
Identified it after push but before merge.
* Update change log
* Apply Column and Row Styles to Existing Cells (#1721)
* Apply Column and Row Styles to Existing Cells
This is a fix for issue #1712.
When a style is applied to an entire row or column, it is currently
only effective for cells which don't already contain a value.
The code needs to iterate through existing cells in the row/column
in order to apply the style to them.
This could be considered a breaking change, however, I believe that
the change makes things operate as users would expect, and that the
existing implementation is incomplete.
The change also removes protected element conditionalStyles from
the Style class. That element is an unused remnant, and can no longer be
set or retrieved - methods getConditionalStyles and setConditionalStyles
actually act on an element in the Worksheet class.
Finally, additional tests are added so that Style, and in fact the
entire Style directory, now has 100% test coverage.
* Scrutinizer Changes
Scrutinizer flagged 6 statements. 5 can be easily corrected.
One is absolutely wrong (it thinks iterating through cells in column
can return null). Let's see if we can satisfy it.
* Remove Exception For CellIterator on Empty Row/Column
For my first attempt at this change, which corrects a bug by updating styles
for non-empty cells when a style is set on a row or column, I wished to make things
more efficient by using setIterateOnlyExistingCells, something which the
existing documentation recommends. This caused an exception to be generated
when the row or column is empty. So I removed that part of the change while I
researched what was going on.
I have completed that research. The existing code does throw an exception
when the row/column is empty and iterateOnlyExistingCells is true. However,
that does not seem like a reasonable action. This situation is analagous to
iterating over an empty array, and that action is legal and does not throw.
The same should apply here. There were no tests for this situation,
and now there are.
I have added additional tests, and coverage for all of RowCellIterator,
ColumnCellIterator, and CellIterator are all now 100%. Some of my new tests
were added in new members, because the existing tests all relied on mocking,
which was not the best choice for the new tests. One of the existing tests
for RowCellIteratorTest (testSeekOutOfRange) was wrong; it issued the expected
exception, but for the wrong reason. I have added an additional test to
ensure that it fails "correctly".
The existing documentation says that the default value for
IterateOnlyExistingCells is true. In fact, the default value is false.
I have corrected the documentation.
* More Scrutinizer
I believe its analysis is incorrect, but this should silence it.
* DocBlock Correction
ColumnCellIterator DocBlock for current indicated it could return null
or Cell, but it can really return only Cell. This had caused Scrutinizer
to complain earlier.
* PHP8 Environment Appears to be Fixed
Cosmetic change to Doc member. I suspect there is a way to rerun all
the tests without another push, but I have been unable to figure out how.
* Update change log
* TextData Coverage and Minor Bug Fixes (#1744)
This had been intended to get 100% coverage for TextData functions, and it does that.
However, some minor bugs requiring source changes arose during testing.
- the Excel CHAR function restricts its argument to 1-255. PhpSpreadsheet CHARACTER
had been allowing 0+. Also, there is no need to test if iconv exists,
since it is part of Composer requirements.
- The DOLLAR function had been returning NUM for invalid arguments. Excel returns VALUE.
Also, negative amounts were not being handled correctly.
- The FIXEDFORMAT function had been returning NUM for invalid arguments. Excel FIXED returns VALUE.
* Replace anti-xss with html purifier (#1751)
* Replace voku/anti-xss with ezyang/htmlpurifier. Despite anti-xss being a smaller footprint dependency, an a better license fit with our MIT license, there are issues with it's automatic it sanitisation of global variables causing side effects
* Additional unit tests for xss in html writer cell comments
* Fix bug #1626 where values of 0 were "rounded" up/down as if they were not 0 (#1627)
* Fix bug where values of 0 were "rounded" up/down as if they were not 0
* Update change log
* Fix for #1612 - SLK Long File Name (#1706)
Issue has been marked stale, but ...
Sylk read sets worksheet title to filename (minus .slk).
If that is >31 characters, PhpSpreadsheet throws Exception.
This change truncates sheet title, as Excel does, to 31 characters.
* Update change log
* worksheet: fix if cellValue does not exist (#1727)
The condition is FALSE if the cell does not exist in the flipped table,
but anyway, it is sent in to a method requiring 'string' type, causing
it to fail.
* fixes #1655 issue (#1656)
Resolve problem with incorrectly defined hyperlinks
* Add 'ps' suffix to printer settings resources IDs (#1690)
* Add 'ps' suffix to printer settings resources IDs
* Update change log
* Fix pixelsToPoints conversion (for HTML col width) (#1733)
* DocBlock Change in Styles/Conditional (#1697)
Scrutinizer reported a minor error in a test involving a module
which I was not changing.
Styles/Conditional function setConditions can take a scalar or an
array as a parameter, but DocBlock says it only expects array.
I did not wish to add the extra module to my PR, but made a note to
self to fix that after PR was installed.
That has now happened, and it makes for a good case for me
to see all the PHP8/Composer2/etc. changes that have happened recently.
* Merge pull request #1698
* Merge pull request #4 from PHPOffice/master
* Restore Omitted Read XML Test
* Fix for bug #1592 (UPDATED) (#1623)
* Fix for Xls when BIFF8 SST (FCh) has bad Shared string length
* Update change log
* Add nightly PHP 8.1 dev to github actions (#1763)
* Fix compatibility with ext-gd on php 8 (#1762)
* CSV - Guess Encoding, Handle Null-string Escape (#1717)
* CSV - Guess Encoding, Handle Null-string Escape
This is in response to issue #1647 (detect CSV character encoding).
First, my tests with mb_detect_encoding indicate that it doesn't work
well enough; regardless, users can always do that on their own
if they deem it useful.
Rolling my own is also troublesome, but I can at least:
a. Check for BOM (UTF-8, UTF-16BE, UTF-16LE, UTF-32BE, UTF-32LE).
b. Do some heuristic tests for each of the above encodings.
c. Fallback to a user-specified encoding (default CP1252)
if a and b don't yield result.
I think this is probably useful enough to include, and relatively
easy to expand if other potential encodings should be considered.
Starting with PHP7.4, fgetcsv allows specification of null string as
escape character in fgetcsv. This is a much better choice than the PHP
(and PhpSpreadsheet) default of backslash in that it handles the file
in the same manner as Excel does. There is one statement in Reader/CSV
which would be adversely affected if the caller so specified (building
a regular expression under the assumption that escape character is
a single character). Fix that statement appropriately and add tests.
* Update changelog
* Update Units of Measure supported by the CONVERT() function (#1768)
Now supports all current UoM in all categories, with both 1- and 2-character multiplier prefixes, and binary multiplier prefixes, including the new Temperature scales
* Changelog for 1.16.0 release
* Fix date tests withut specified year for current year 2021 (#1774)
* Mrand of zero to any multiple should return 0 (#1773)
* Inherited Scrutinizer Recommendations - 3 of 3
I tried to sync my fork with the main project, as I have done several times before.
However, the GitHub interface to do this had changed, and it appears that I
did not make the optimal selection when I had a choice. Consequently, all
the merges that happened to base between the last time I synchronized and this time
appear to be part of any PR that I push. "Files changed" remains correct for
my new PRs, but there appear to be many more commits involved than is actually the
case. I will, at some point, delete and re-create my fork, and pay much closer
attention in future when I want to sync my fork with the main project.
Because of this set-up, Scrutinizer reports flaws in code that I haven't
actually changed in my PRs #1799 and #1800.
It still passes them, but, as long as I'm aware of the problems,
I may as well attempt to correct them. The following are not part of those PRs:
- 5 problems spread over 4 different members
- 12 problems in Calculation/Engineering
- 15 problems in Reader/XML
I shall attempt to resolve these via 3 separate PRs,
of which this is the third.
* The Usual
Fixed some Scrutinizer problems, some new ones popped up.
* More Scrutinizer
I think it's wrong in a lot of these cases. Although I am working
around all of them, I intend to file a bug report with them.
Co-authored-by: Mark Baker <[email protected]>
Co-authored-by: Adrien Crivelli <[email protected]>
Co-authored-by: Ryan McAllen <[email protected]>
Co-authored-by: Flinsch <[email protected]>
Co-authored-by: Jan Sverre Riksfjord <[email protected]>
Co-authored-by: Max Kalyabin <[email protected]>
Co-authored-by: Sébastien Despont <[email protected]>
Co-authored-by: Guilliam Xavier <[email protected]>
Co-authored-by: Gianluca Giovinazzo <[email protected]>
Co-authored-by: Alexander M. Turek <[email protected]>
Co-authored-by: Martins Sipenko <[email protected]>
* Fix/1674 (#1688)
* Treat inline strings like strings in Open Document because it has no specific inline-string format
* implement data-type error
Co-authored-by: Mark Baker <[email protected]>
* Fixed reading XSLS style alignments from XML (#1710)
The attribute `$alignmentXml` given to the private method `readAlignmentStyle` is the alignment XML tag that contains the alignment data itself. But inside that method all data are read from another `alignment` XML tag within that given tag. This redundant child-node access resulted in the following error- / notice-message: `PHP Notice: Trying to access array offset on value of type null in /foo/bar/baz/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Reader/Xlsx/Styles.php on line 146`
These changes remove the redundant child-node access in …
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is:
Checklist:
Why this change is needed?
Reader/Xml is now 100% covered.
File templates/Excel2003XMLTest.xml, used in some tests, is not
readable by a current version of Excel. I have substituted a new file
excel2003.xml to be used in its place. I have not deleted the original
in case someone in future (possibly me) wants to see what it needs to
make it usable.
There are minimal code changes.
are deleted.
and made case insensitive.
was attempted and threw exception).
In theory, encodings other than UTF-8 were supported.
In fact, I was unable to get SecurityScanner to pass any xml which is
not UTF-8. Eliminating the assumption that strings might not be UTF-8
allowed much of the code to be greatly simplified.
After that, I added some code that would permit the use of
some ASCII-compatible encodings (there is a test of ISO-8859-1).
It would be more difficult to handle other encodings (such as UTF-16).
I am not convinced that even the ISO-8859 effort is worth it,
but am willing to investigate either expanding or eliminating
non-UTF8 support.
I added a number of tests, creating an Xml directory, and moving
XmlTest to that directory.
Pull Request had problems reading old invalid sample in the code
coverage phase, not in any of the other test phases, and not in
the code coverage phase on my local machine.
As it turns out, aside from being invalid, the sample
is much larger than any of the other samples. Tests have been
adjusted accordingly.