From bd05c590e3e9eca5d5a420f6b68a905d7518ff7c Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Thu, 26 Nov 2020 11:10:52 +0900 Subject: [PATCH 01/30] Drop Travis --- .gitattributes | 1 - .github/workflows/main.yml | 2 +- .travis.yml | 34 ------------------- README.md | 2 +- .../PhpSpreadsheetTests/Helper/SampleTest.php | 2 +- .../Reader/Xml/XmlLoadTest.php | 7 +--- 6 files changed, 4 insertions(+), 44 deletions(-) delete mode 100644 .travis.yml diff --git a/.gitattributes b/.gitattributes index 0186deae..19c90f40 100644 --- a/.gitattributes +++ b/.gitattributes @@ -5,7 +5,6 @@ /.php_cs.dist export-ignore /.sami.php export-ignore /.scrutinizer.yml export-ignore -/.travis.yml export-ignore /CHANGELOG.PHPExcel.md export-ignore /bin export-ignore /composer.lock export-ignore diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 88168fa0..febc244b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,4 +1,4 @@ -name: Build +name: main on: [ push, pull_request ] jobs: test: diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 6eb59290..00000000 --- a/.travis.yml +++ /dev/null @@ -1,34 +0,0 @@ -language: php -dist: bionic - -php: - - 7.2 - - 7.3 - - 7.4 - - nightly - -cache: - directories: - - vendor - - $HOME/.composer/cache - -before_script: - # Deactivate xdebug - - if [[ $TRAVIS_PHP_VERSION != nightly ]]; then phpenv config-rm xdebug.ini; fi - - if [[ $TRAVIS_PHP_VERSION == nightly ]]; then rm composer.lock; fi - - composer install --ignore-platform-reqs - -script: - - ./vendor/bin/phpunit --color=always --coverage-text - -allow_failures: - - php: nightly - -jobs: - include: - - - stage: Code style - php: 7.4 - script: - - ./vendor/bin/php-cs-fixer fix --diff --verbose --dry-run - - ./vendor/bin/phpcs diff --git a/README.md b/README.md index 0cef8326..2a94e0d3 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # PhpSpreadsheet -[![Build Status](https://travis-ci.org/PHPOffice/PhpSpreadsheet.svg?branch=master)](https://travis-ci.org/PHPOffice/PhpSpreadsheet) +[![Build Status](https://github.com/PHPOffice/PhpSpreadsheet/workflows/main/badge.svg)](https://github.com/PHPOffice/PhpSpreadsheet/actions) [![Code Quality](https://scrutinizer-ci.com/g/PHPOffice/PhpSpreadsheet/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/PHPOffice/PhpSpreadsheet/?branch=master) [![Code Coverage](https://scrutinizer-ci.com/g/PHPOffice/PhpSpreadsheet/badges/coverage.png?b=master)](https://scrutinizer-ci.com/g/PHPOffice/PhpSpreadsheet/?branch=master) [![Total Downloads](https://img.shields.io/packagist/dt/PHPOffice/PhpSpreadsheet)](https://packagist.org/packages/phpoffice/phpspreadsheet) diff --git a/tests/PhpSpreadsheetTests/Helper/SampleTest.php b/tests/PhpSpreadsheetTests/Helper/SampleTest.php index 369b205e..8fd6bb47 100644 --- a/tests/PhpSpreadsheetTests/Helper/SampleTest.php +++ b/tests/PhpSpreadsheetTests/Helper/SampleTest.php @@ -43,7 +43,7 @@ class SampleTest extends TestCase } // Unfortunately some tests are too long be ran with code-coverage - // analysis on Travis, so we need to exclude them + // analysis on GitHub Actions, so we need to exclude them global $argv; if (in_array('--coverage-clover', $argv)) { $tooLongToBeCovered = [ diff --git a/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php b/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php index 03f977f5..969571f2 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php @@ -82,14 +82,9 @@ class XmlLoadTest extends TestCase public function testLoadUnusableSample(): void { // Sample spreadsheet is not readable by Excel. - // But PhpSpreadsheet can load it except for coverage test. - //global $argv; - //if (in_array('--coverage-clover', $argv)) { - // self::markTestSkipped('Mysterious Travis coverage failure IOFactoryTest'); - //} + // But PhpSpreadsheet can load it. $filename = __DIR__ . '/../../../..' - //. '/samples/templates/Excel2003XMLTest.xml'; . '/samples/templates/excel2003.short.bad.xml'; $reader = new Xml(); $spreadsheet = $reader->load($filename); From ba1ce8b8ecb1b2587506d6d1b911b21e8958a91f Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Thu, 26 Nov 2020 12:44:07 +0900 Subject: [PATCH 02/30] Automatic GitHub releases from git tags --- .github/workflows/main.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index febc244b..f4e13a65 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -148,3 +148,25 @@ jobs: ./vendor/bin/phpunit --coverage-clover coverage-clover.xml curl -LO https://scrutinizer-ci.com/ocular.phar php ocular.phar code-coverage:upload --format=php-clover coverage-clover.xml + + release: + runs-on: ubuntu-latest + if: github.event_name == 'push' && contains(github.ref, 'refs/tags/') + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.ref }} # Otherwise our annotated tag is not fetched and we cannot get correct version + + # Create release + - name: Get release info + id: release-info + run: | + echo "::set-output name=subject::$(git tag --format '%(contents:subject)' --points-at)" + git tag --format '%(contents:body)' --points-at > release-body.txt + - uses: actions/create-release@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token + with: + tag_name: ${{ github.ref }} + release_name: ${{ steps.release-info.outputs.subject }} + body_path: release-body.txt From 1a0aab1a4ffc1f027f4e78b2d3c0050b1de4a4ce Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Fri, 27 Nov 2020 06:50:01 -0800 Subject: [PATCH 03/30] 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 --- samples/Reader/sampleData/example1xls | Bin 0 -> 22528 bytes .../DefinedNameFormulaTest.php | 21 +++++ tests/PhpSpreadsheetTests/DefinedNameTest.php | 82 ++++++++++++++++++ .../DocumentGeneratorTest.php | 10 +++ .../Functional/CommentsTest.php | 15 +++- tests/PhpSpreadsheetTests/IOFactoryTest.php | 41 +++++++++ 6 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 samples/Reader/sampleData/example1xls diff --git a/samples/Reader/sampleData/example1xls b/samples/Reader/sampleData/example1xls new file mode 100644 index 0000000000000000000000000000000000000000..bd9bb110b9a41b0aab816ba9e20682ee5bb43aea GIT binary patch literal 22528 zcmeHPd2AfldH-g)%R?l!yhO^f#EePVk|=F`4@lQe*F_)S@#2pguA6q-dJdo`YODG=K%}1 z3Fr$81Z}D;$orXW1$77wy4q{--(IMSdscj7mxJl9htIz0_w= z-J8|xfciBWBvDH%M_NRwHFb8Ry(YCcxmHrrDbn}JzW#0f{X=)2AGD9X$qktQYpZAv zo%C`KS2OfV1G*wCynU#;JfPlWzcfnkVvdLuWxE|K%aF(gRzBdWCKm>cE)_7K_6_!d zc407cVX!y3O}5A;7Ob$HuX-ka5&vDXO)w;?D}n-%x(udL-N;C#!d_C%L#@(Y|*@B6#lU%HJ5}cb1XERdT5u!HBMn|8z}! zgROTW(YH!gr6jpd$7coa_DQOOr+T&Z{7g;xrd@r)CCh|)_rr;#w9VBmFmIo$*Supc zsd?vIgXS_Pns?2m79P>P;0esiS1?&cT4i6hoOMr4cwX7C|5xH6GNzPFTp)Q39vOk_)iyhnLnp`69w**`u(!U6wUx#X9D zxYsXS0{x(0_6o=k6?6G4DmgrZa-<*07kp$#^HVH-EI;W%o8$hY9+gIc6Vv0_5-4NY zD=c^4jCau+2mYi#<@rFI%6p{}bvm6bUS`#2iup@u3`qCq$IE^Rs0SwU)05sTn9dHK z%+7*zE?b=RJcxa8rZ9tS^kRXn&?5_uHT(R_K<>{@&jLVOP~u+CFJ1Nk9`vt*cBt%4 z0XUo|;}IAe*->Z>-0^G?z=?}KC60MzZ3guAihxi0uqg6VGnchlr?VxpohgnNJZhRR z6!O_Alt1TBlJKB+vBY+dUb#~6rtz?Mrd;r7kbN_kE&<-3yP7TK@Nl5y<#KG}!EC8S zy$=;9A@Q(3Q7q=!uSfl2&M)HOn2*NKLd5ZzGDV&!!E~&BY$lgOQ}CqfYN?Yx`k@T5 zr~Rp^S&;6}7d_~Xa;hvPpY=-DRJn5;lvm#Y$)l%FxMLH3(JSZ2T^%+TF)KD7^K)LQ zxWz?_Pt~Fi@7XmxIK1cfJGN}5t!A?Y_jvZv+1jkbyTCfUdkfk+IzEZ^?Df17fPH=u zda6D0?2vo4r}s}yWq~%0od$X=e+A9jKRxXcI_-}m#gIXJ&iN%AU5C*^ZyMcKbPtq# zOr%o<*y#pT@os_cqX%kp?x51Uc5GRY_u{zo0cg~Msr^0V9`Y?1{LdkuK>qK@zleMf zmV$plK8yU;CXp{A&m#XJ@@J6`VYB^b-W%_;O+3EDdTrVegTK6cj92Z zM|Q||tu+`g~I@Jmo`5Km^*DA9T9A=x~{E4%>@Gz>u!UT@G81RFNu4IbPGK6HwBk6!>t(fB_MEg!KJGqI98U?+vfNptIPNS_ z9P5wbZ~|V0d!3GCgdWYmuKT9L`}+KQGDtI1OrM^aM>_XdCMo2n(miOk$k*Z4rG0J^ z2mDA+{$2(yVOYVznQV=`R`}~*`^#6qeYe3R!Pj&) z&#LvXTLWbCCNKajU-DpdF4V~=RoDO`8=BA}B!c;k2I*~KZ`QvIr_%>v+^68qy&q@! z+hn7j;qOj#$n$J~WS9hD0|e2c2xe@G{0McVaQNq)s2jBy8at01R68YKah?z-#exe9 zBgw>E6;DMIV}cvHM(Cl8LV~a-$UlnQ)LO|&0{{E<(5H&h+QT=_US zpIj&M`3?Vody2gSQ#c`}eaqOBXj*~Nr z<7ABDsB09*X%fY8`HkXuDUIUTM^PM|j8Pn~5m8)g4ICH$YCbM<47A>>BX+SgkXkg8 z&?S^tWWtRZrfrH{i)mOFlr+4CBZU%cvG^KVo0jO(bZy4aQUPtfVqH!9l-7&ht{2LA zb);G*pW8A`kj&RdhQrDv%}O#HjR8lWO5UjHZJKTlWbiFD%MB=Yo2J_}9a4^sSl(7C zM=d>AvMaM1+H8a$-SDl|(pC?0*xC(WOTcIKSX#T`YYq5@RNk8_yO)dAn_ zioHYAcWJs?(>>sO8S`irhVxoyHMWIrY*+mC1;5Q=e!9#;-x|xTT}sWJx3bLM$kM@` z^4U!y8*sRfy+vx@C5hd@stLq#35vZ%#wuK9t#^pe3>N0|O1(cT@@mlcWU@u3_H7-X{98Z5y98V@u z9CyYjjyrV}$IDR^$BR%D$BR%D#}j81$M!^Vya+{cya+{cya+{cZMYOfaqTs59W`*B zHE>-uaBFJdx+6IHRv274f%F^rbdjzEH~1M37yT=)+!xXtLi#H86gi-z(Hi4hq6;Lg zorixux-<#ySiw35b^y1M!-4X9zU_;@cuaYXnr0(gf(Pkj6{V-2r4S+4M59 zoViN2hY)q?2qEf%PYVN1>cV%zrW|z{2_fn-5JJ=?6GGHwG=!+j-T*R}d%k0)Oyi5= zX~sa*g>%I~)CFF}0HQAZjK|QZ%k~hWF872Gb=em})MbANQI`V&#Cem3f6>UK$1e?^ zpM|K)`VgWn@WE*9^yjAOISU}_a&HJxmxCchT@Hm1b-6u+sLSC1;(SfR>1SlpahAr2 zvJiE-C4{I;UkFi`{t%)rbi~n0DO}alcoT0S>T)!MsLQbsqAte+h<@!fuGXepJ-nA` zyz@5@*9%w1kaAu$LQgmNnR&shzbo$v>Ct&%oGiBn(welLw}lYvwIjMoofNphYFnDr z=9@!^?K~MmZ0D&EqP`U=Z0G5a#&+6y!C9Whx2UED&dbV(<~&K`vr$W<&9{XRZ9W)6 z)a6VFQJ4Eeh`MA#h`Nl15OuNhf-6O3MDydoG(TC@wl}LTc3w2AU4}v$b>aIv_INX_ zUoC~Y*m=>c{rEu0NnPx`Fiw=S0nIqGc7zah*%?B#%dQZjUF?W%(LTB(q*0f1Aw*p& zQfL=DFIwQkPRoNKC++f30I?q{Bf3@X;)XQpvO9#T=S8dPLZ>k8(yDf`^P*Mf#rcp% zT`E$ji=AD@8T4?#$xqKKGl8SO0&(whdlUbLYGH~5+TfcsclE(DU=RRcR>+f|Ys zvF&ONJ7U|RePzUQ=c}y29V*4H!5u0k6V%Y5QtVjkP$_n-aj&e5l1`OkM@gqjv8!jN zO0lD)Q>EC|le=rBf4fwQ?cXkyV*9sCrPxu@rBZDFazC#0FFwcwWP4_fO0hk&My1%E zS))>H&vYv%*Ql4VD?EZl=4mH`_ZjxLUKT?x_5k~P6ZaeBAvh*W{v)=#6uOpO&834O ziHf--ax3AHV3oqm7dz{e{8LKa!26q&lUO2+dR~9xcmM6jCnnmy@av7Tb<|y*L%N(&O!L6p8oB4uH$nYVhh9;h%FFXAhtkkf!G4E1!4=t7Kkkn zTOhVTY=K1KpMmVX?P{(z?tdH4S_#A<&4eF+S_!>2bjt1QBl$q>zZQ3-kZx?|B_ z=`O+SSc2IpEhx>}>4~6Z{LSs;H2=(ZT7|}r1lS}Fu?1oa#1@Dx5L+O&Kx~270 z&+kH9gUGu+{Q+wc`M*K=Q5-*oTaU=k-gx8Y$7wDi>3zui5jP^N32#HNBGNfEk19A@cW)HxSze}YRfNv z%VA$y+8K5Rz)oI{*HH(EbGJk`mAEQOCt`LB(rgZVcaE7NE9|DgT1RMr3A!Bl4j literal 0 HcmV?d00001 diff --git a/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php b/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php index 50304991..14843091 100644 --- a/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php +++ b/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests; use PhpOffice\PhpSpreadsheet\DefinedName; +use PhpOffice\PhpSpreadsheet\NamedFormula; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; @@ -164,4 +165,24 @@ class DefinedNameFormulaTest extends TestCase 'utf-8 named ranges in a formula' => ['Здравствуй+мир', true], ]; } + + public function testEmptyNamedFormula(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $spreadSheet = new Spreadsheet(); + $workSheet1 = $spreadSheet->getActiveSheet(); + new NamedFormula('namedformula', $workSheet1); + } + + public function testChangeFormula(): void + { + $spreadSheet = new Spreadsheet(); + $workSheet1 = $spreadSheet->getActiveSheet(); + $namedFormula = new NamedFormula('namedformula', $workSheet1, '=1'); + self::assertEquals('=1', $namedFormula->getFormula()); + $namedFormula->setFormula('=2'); + self::assertEquals('=2', $namedFormula->getFormula()); + $namedFormula->setFormula(''); + self::assertEquals('=2', $namedFormula->getFormula()); + } } diff --git a/tests/PhpSpreadsheetTests/DefinedNameTest.php b/tests/PhpSpreadsheetTests/DefinedNameTest.php index 3f5b75a0..c199e7f2 100644 --- a/tests/PhpSpreadsheetTests/DefinedNameTest.php +++ b/tests/PhpSpreadsheetTests/DefinedNameTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests; use PhpOffice\PhpSpreadsheet\DefinedName; +use PhpOffice\PhpSpreadsheet\NamedRange; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; use PHPUnit\Framework\TestCase; @@ -121,4 +122,85 @@ class DefinedNameTest extends TestCase $this->spreadsheet->getDefinedName('foo')->getValue() ); } + + public function testDefinedNameNoWorksheetNoScope(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + new NamedRange('xyz'); + } + + public function testSetAndGetRange(): void + { + $this->spreadsheet->addDefinedName( + DefinedName::createInstance('xyz', $this->spreadsheet->getActiveSheet(), 'A1') + ); + + $namedRange = $this->spreadsheet->getDefinedName('XYZ'); + self::assertInstanceOf(NamedRange::class, $namedRange); + if ($namedRange instanceof NamedRange) { + self::assertEquals('A1', $namedRange->getRange()); + self::assertEquals('A1', $namedRange->getValue()); + $namedRange->setRange('A2'); + self::assertEquals('A2', $namedRange->getValue()); + } + } + + public function testChangeWorksheet(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('xyz', $sheet2, '$A$1'); + $namedRange->setWorksheet($sheet1); + $this->spreadsheet->addNamedRange($namedRange); + $sheet1->getCell('B2')->setValue('=XYZ'); + self::assertEquals(1, $sheet1->getCell('B2')->getCalculatedValue()); + $sheet2->getCell('B2')->setValue('=XYZ'); + self::assertEquals(1, $sheet2->getCell('B2')->getCalculatedValue()); + } + + public function testLocalOnly(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('abc', $sheet2, '$A$1'); + $namedRange->setWorksheet($sheet1)->setLocalOnly(true); + $this->spreadsheet->addNamedRange($namedRange); + $sheet1->getCell('C2')->setValue('=ABC'); + self::assertEquals(1, $sheet1->getCell('C2')->getCalculatedValue()); + $sheet2->getCell('C2')->setValue('=ABC'); + self::assertEquals('#NAME?', $sheet2->getCell('C2')->getCalculatedValue()); + } + + public function testScope(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('abc', $sheet2, '$A$1'); + $namedRange->setScope($sheet1); + $this->spreadsheet->addNamedRange($namedRange); + $sheet1->getCell('C2')->setValue('=ABC'); + self::assertEquals(2, $sheet1->getCell('C2')->getCalculatedValue()); + $sheet2->getCell('C2')->setValue('=ABC'); + self::assertEquals('#NAME?', $sheet2->getCell('C2')->getCalculatedValue()); + } + + public function testClone(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('abc', $sheet2, '$A$1'); + $namedRangeClone = clone $namedRange; + $ss1 = $namedRange->getWorksheet(); + $ss2 = $namedRangeClone->getWorksheet(); + self::assertNotSame($ss1, $ss2); + self::assertEquals($ss1->getTitle(), $ss2->getTitle()); + } } diff --git a/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php b/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php index 8a34f1c0..45000f8e 100644 --- a/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php +++ b/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php @@ -7,6 +7,7 @@ use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Calculation\Logical; use PhpOffice\PhpSpreadsheet\DocumentGenerator; use PHPUnit\Framework\TestCase; +use UnexpectedValueException; class DocumentGeneratorTest extends TestCase { @@ -137,4 +138,13 @@ EXPECTED ], ]; } + + public function testGenerateFunctionBadArray(): void + { + $this->expectException(UnexpectedValueException::class); + $phpSpreadsheetFunctions = [ + 'ABS' => ['category' => Cat::CATEGORY_MATH_AND_TRIG, 'functionCall' => 1], + ]; + DocumentGenerator::generateFunctionListByName($phpSpreadsheetFunctions); + } } diff --git a/tests/PhpSpreadsheetTests/Functional/CommentsTest.php b/tests/PhpSpreadsheetTests/Functional/CommentsTest.php index 6f1c5340..5ba4e7c8 100644 --- a/tests/PhpSpreadsheetTests/Functional/CommentsTest.php +++ b/tests/PhpSpreadsheetTests/Functional/CommentsTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Functional; use PhpOffice\PhpSpreadsheet\Spreadsheet; +use PhpOffice\PhpSpreadsheet\Style\Alignment; class CommentsTest extends AbstractFunctional { @@ -35,10 +36,22 @@ class CommentsTest extends AbstractFunctional $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format); - $commentsLoaded = $reloadedSpreadsheet->getSheet(0)->getComments(); + $sheet = $reloadedSpreadsheet->getSheet(0); + $commentsLoaded = $sheet->getComments(); self::assertCount(1, $commentsLoaded); $commentCoordinate = key($commentsLoaded); self::assertSame('E10', $commentCoordinate); + $comment = $commentsLoaded[$commentCoordinate]; + self::assertEquals('Comment to test', (string) $comment); + $commentClone = clone $comment; + self::assertEquals($comment, $commentClone); + self::assertNotSame($comment, $commentClone); + if ($format === 'Xlsx') { + self::assertEquals('feb0c24b880a8130262dadf801f85e94', $comment->getHashCode()); + self::assertEquals(Alignment::HORIZONTAL_GENERAL, $comment->getAlignment()); + $comment->setAlignment(Alignment::HORIZONTAL_RIGHT); + self::assertEquals(Alignment::HORIZONTAL_RIGHT, $comment->getAlignment()); + } } } diff --git a/tests/PhpSpreadsheetTests/IOFactoryTest.php b/tests/PhpSpreadsheetTests/IOFactoryTest.php index f9512a34..9b07ad33 100644 --- a/tests/PhpSpreadsheetTests/IOFactoryTest.php +++ b/tests/PhpSpreadsheetTests/IOFactoryTest.php @@ -161,4 +161,45 @@ class IOFactoryTest extends TestCase IOFactory::registerReader('foo', 'bar'); } + + public function testCreateInvalidWriter(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Writer\Exception::class); + $spreadsheet = new Spreadsheet(); + IOFactory::createWriter($spreadsheet, 'bad'); + } + + public function testCreateInvalidReader(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class); + IOFactory::createReader('bad'); + } + + public function testCreateReaderUnknownExtension(): void + { + $filename = 'samples/Reader/sampleData/example1.tsv'; + $reader = IOFactory::createReaderForFile($filename); + self::assertEquals('PhpOffice\\PhpSpreadsheet\\Reader\\Csv', get_class($reader)); + } + + public function testCreateReaderCsvExtension(): void + { + $filename = 'samples/Reader/sampleData/example1.csv'; + $reader = IOFactory::createReaderForFile($filename); + self::assertEquals('PhpOffice\\PhpSpreadsheet\\Reader\\Csv', get_class($reader)); + } + + public function testCreateReaderNoExtension(): void + { + $filename = 'samples/Reader/sampleData/example1xls'; + $reader = IOFactory::createReaderForFile($filename); + self::assertEquals('PhpOffice\\PhpSpreadsheet\\Reader\\Xls', get_class($reader)); + } + + public function testCreateReaderNotSpreadsheet(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class); + $filename = __FILE__; + $reader = IOFactory::createReaderForFile($filename); + } } From 6b4feb6142e3422554ae058c4983dccfc3a555fc Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Fri, 27 Nov 2020 07:16:23 -0800 Subject: [PATCH 04/30] Changes for Scrutinizer Two changes to fix minor problems reported by Scrutinizer. --- tests/PhpSpreadsheetTests/DefinedNameTest.php | 10 ++++------ tests/PhpSpreadsheetTests/IOFactoryTest.php | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/PhpSpreadsheetTests/DefinedNameTest.php b/tests/PhpSpreadsheetTests/DefinedNameTest.php index c199e7f2..8a411775 100644 --- a/tests/PhpSpreadsheetTests/DefinedNameTest.php +++ b/tests/PhpSpreadsheetTests/DefinedNameTest.php @@ -137,12 +137,10 @@ class DefinedNameTest extends TestCase $namedRange = $this->spreadsheet->getDefinedName('XYZ'); self::assertInstanceOf(NamedRange::class, $namedRange); - if ($namedRange instanceof NamedRange) { - self::assertEquals('A1', $namedRange->getRange()); - self::assertEquals('A1', $namedRange->getValue()); - $namedRange->setRange('A2'); - self::assertEquals('A2', $namedRange->getValue()); - } + self::assertEquals('A1', $namedRange->getRange()); + self::assertEquals('A1', $namedRange->getValue()); + $namedRange->setRange('A2'); + self::assertEquals('A2', $namedRange->getValue()); } public function testChangeWorksheet(): void diff --git a/tests/PhpSpreadsheetTests/IOFactoryTest.php b/tests/PhpSpreadsheetTests/IOFactoryTest.php index 9b07ad33..886fcb36 100644 --- a/tests/PhpSpreadsheetTests/IOFactoryTest.php +++ b/tests/PhpSpreadsheetTests/IOFactoryTest.php @@ -200,6 +200,6 @@ class IOFactoryTest extends TestCase { $this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class); $filename = __FILE__; - $reader = IOFactory::createReaderForFile($filename); + IOFactory::createReaderForFile($filename); } } From 7247e3b5ac9c652e4f81ab4247642812d279c43d Mon Sep 17 00:00:00 2001 From: Ryan McAllen Date: Mon, 7 Dec 2020 10:17:56 -0500 Subject: [PATCH 05/30] Spelling: Tou -> You --- src/PhpSpreadsheet/NamedFormula.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/NamedFormula.php b/src/PhpSpreadsheet/NamedFormula.php index ffb1c9b4..eeddbbcb 100644 --- a/src/PhpSpreadsheet/NamedFormula.php +++ b/src/PhpSpreadsheet/NamedFormula.php @@ -18,7 +18,7 @@ class NamedFormula extends DefinedName ) { // Validate data if (empty($formula)) { - throw new Exception('Tou must specify a Formula value for a Named Formula'); + throw new Exception('You must specify a Formula value for a Named Formula'); } parent::__construct($name, $worksheet, $formula, $localOnly, $scope); } From ce7863570a99b5d44c3be7c24333a7db8ac672ec Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 10 Dec 2020 09:01:08 -0800 Subject: [PATCH 06/30] 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. --- src/PhpSpreadsheet/Spreadsheet.php | 2 +- tests/PhpSpreadsheetTests/SpreadsheetTest.php | 143 +++++++++++++++++- 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/src/PhpSpreadsheet/Spreadsheet.php b/src/PhpSpreadsheet/Spreadsheet.php index 19c11526..c8e8f72c 100644 --- a/src/PhpSpreadsheet/Spreadsheet.php +++ b/src/PhpSpreadsheet/Spreadsheet.php @@ -665,7 +665,7 @@ class Spreadsheet // Adjust active sheet index if necessary if ( ($this->activeSheetIndex >= $pIndex) && - ($pIndex > count($this->workSheetCollection) - 1) + ($this->activeSheetIndex > 0 || $numSheets <= 1) ) { --$this->activeSheetIndex; } diff --git a/tests/PhpSpreadsheetTests/SpreadsheetTest.php b/tests/PhpSpreadsheetTests/SpreadsheetTest.php index 05fbe1b5..129bea7c 100644 --- a/tests/PhpSpreadsheetTests/SpreadsheetTest.php +++ b/tests/PhpSpreadsheetTests/SpreadsheetTest.php @@ -51,6 +51,147 @@ class SpreadsheetTest extends TestCase */ public function testGetSheetByName($index, $sheetName): void { - self::assertEquals($this->object->getSheet($index), $this->object->getSheetByName($sheetName)); + self::assertSame($this->object->getSheet($index), $this->object->getSheetByName($sheetName)); + } + + public function testAddSheetDuplicateTitle(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $sheet = new Worksheet(); + $sheet->setTitle('someSheet2'); + $this->object->addSheet($sheet); + } + + public function testAddSheetNoAdjustActive(): void + { + $this->object->setActiveSheetIndex(2); + self::assertEquals(2, $this->object->getActiveSheetIndex()); + $sheet = new Worksheet(); + $sheet->setTitle('someSheet4'); + $this->object->addSheet($sheet); + self::assertEquals(2, $this->object->getActiveSheetIndex()); + } + + public function testAddSheetAdjustActive(): void + { + $this->object->setActiveSheetIndex(2); + self::assertEquals(2, $this->object->getActiveSheetIndex()); + $sheet = new Worksheet(); + $sheet->setTitle('someSheet0'); + $this->object->addSheet($sheet, 0); + self::assertEquals(3, $this->object->getActiveSheetIndex()); + } + + public function testRemoveSheetIndexTooHigh(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $this->object->removeSheetByIndex(4); + } + + public function testRemoveSheetNoAdjustActive(): void + { + $this->object->setActiveSheetIndex(1); + self::assertEquals(1, $this->object->getActiveSheetIndex()); + $this->object->removeSheetByIndex(2); + self::assertEquals(1, $this->object->getActiveSheetIndex()); + } + + public function testRemoveSheetAdjustActive(): void + { + $this->object->setActiveSheetIndex(2); + self::assertEquals(2, $this->object->getActiveSheetIndex()); + $this->object->removeSheetByIndex(1); + self::assertEquals(1, $this->object->getActiveSheetIndex()); + } + + public function testGetSheetIndexTooHigh(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $this->object->getSheet(4); + } + + public function testGetIndexNonExistent(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $sheet = new Worksheet(); + $sheet->setTitle('someSheet4'); + $this->object->getIndex($sheet); + } + + public function testSetIndexByName(): void + { + $this->object->setIndexByName('someSheet1', 1); + self::assertEquals('someSheet2', $this->object->getSheet(0)->getTitle()); + self::assertEquals('someSheet1', $this->object->getSheet(1)->getTitle()); + self::assertEquals('someSheet 3', $this->object->getSheet(2)->getTitle()); + } + + public function testRemoveAllSheets(): void + { + $this->object->setActiveSheetIndex(2); + self::assertEquals(2, $this->object->getActiveSheetIndex()); + $this->object->removeSheetByIndex(0); + self::assertEquals(1, $this->object->getActiveSheetIndex()); + $this->object->removeSheetByIndex(0); + self::assertEquals(0, $this->object->getActiveSheetIndex()); + $this->object->removeSheetByIndex(0); + self::assertEquals(-1, $this->object->getActiveSheetIndex()); + $sheet = new Worksheet(); + $sheet->setTitle('someSheet4'); + $this->object->addSheet($sheet); + self::assertEquals(0, $this->object->getActiveSheetIndex()); + } + + public function testBug1735(): void + { + $spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet(); + $spreadsheet->createSheet()->setTitle('addedsheet'); + $spreadsheet->setActiveSheetIndex(1); + $spreadsheet->removeSheetByIndex(0); + $sheet = $spreadsheet->getActiveSheet(); + self::assertEquals('addedsheet', $sheet->getTitle()); + } + + public function testSetActiveSheetIndexTooHigh(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $this->object->setActiveSheetIndex(4); + } + + public function testSetActiveSheetNoSuchName(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $this->object->setActiveSheetIndexByName('unknown'); + } + + public function testAddExternal(): void + { + $spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet(); + $sheet = $spreadsheet->createSheet()->setTitle('someSheet19'); + $sheet->getCell('A1')->setValue(1); + $sheet->getCell('A1')->getStyle()->getFont()->setBold(true); + $sheet->getCell('B1')->getStyle()->getFont()->setSuperscript(true); + $sheet->getCell('C1')->getStyle()->getFont()->setSubscript(true); + self::assertCount(4, $spreadsheet->getCellXfCollection()); + self::assertEquals(1, $sheet->getCell('A1')->getXfIndex()); + $this->object->getActiveSheet()->getCell('A1')->getStyle()->getFont()->setBold(true); + self::assertCount(2, $this->object->getCellXfCollection()); + $sheet3 = $this->object->addExternalSheet($sheet); + self::assertCount(6, $this->object->getCellXfCollection()); + self::assertEquals('someSheet19', $sheet3->getTitle()); + self::assertEquals(1, $sheet3->getCell('A1')->getValue()); + self::assertTrue($sheet3->getCell('A1')->getStyle()->getFont()->getBold()); + // Prove Xf index changed although style is same. + self::assertEquals(3, $sheet3->getCell('A1')->getXfIndex()); + } + + public function testAddExternalDuplicateName(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet(); + $sheet = $spreadsheet->createSheet()->setTitle('someSheet1'); + $sheet->getCell('A1')->setValue(1); + $sheet->getCell('A1')->getStyle()->getFont()->setBold(true); + $this->object->addExternalSheet($sheet); } } From ff04c8502a3f170f063deef13af9c623264ba496 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 10 Dec 2020 18:05:44 +0100 Subject: [PATCH 07/30] Update change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b366f11..3ba28c26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Fix for issue [#1735](https://github.com/PHPOffice/PhpSpreadsheet/issues/1735) Incorrect activeSheetIndex after RemoveSheetByIndex - [PR #1743](https://github.com/PHPOffice/PhpSpreadsheet/pull/1743) - Ensure that the list of shared formulae is maintained when an xlsx file is chunked with readFilter[Issue #169](https://github.com/PHPOffice/PhpSpreadsheet/issues/1669). - Fix for notice during accessing "cached magnification factor" offset [#1354](https://github.com/PHPOffice/PhpSpreadsheet/pull/1354) From 497a934374a8a084414d08644dab828b527ac097 Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 10 Dec 2020 09:08:10 -0800 Subject: [PATCH 08/30] 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. --- src/PhpSpreadsheet/Reader/Xlsx.php | 7 ++++-- .../Reader/Xlsx/NamedRangeTest.php | 22 ++++++++++++++++++ tests/data/Reader/XLSX/bug1686b.xlsx | Bin 0 -> 11740 bytes 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php create mode 100644 tests/data/Reader/XLSX/bug1686b.xlsx diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index 3a75edf6..5dc48ff8 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -1280,7 +1280,7 @@ class Xlsx extends BaseReader } // Valid range? - if (stripos((string) $definedName, '#REF!') !== false || $extractedRange == '') { + if ($extractedRange == '') { continue; } @@ -1350,7 +1350,7 @@ class Xlsx extends BaseReader $extractedRange = (string) $definedName; // Valid range? - if (stripos((string) $definedName, '#REF!') !== false || $extractedRange == '') { + if ($extractedRange == '') { continue; } @@ -1398,6 +1398,9 @@ class Xlsx extends BaseReader $locatedSheet = $excel->getSheetByName($extractedSheetName); } + if ($locatedSheet === null && !DefinedName::testIfFormula($definedRange)) { + $definedRange = '#REF!'; + } $excel->addDefinedName(DefinedName::createInstance((string) $definedName['name'], $locatedSheet, $definedRange, false)); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php new file mode 100644 index 00000000..e4090d98 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/NamedRangeTest.php @@ -0,0 +1,22 @@ +load($xlsxFile); + $sheet = $spreadsheet->getActiveSheet(); + self::assertEquals(2.1, $sheet->getCell('A1')->getCalculatedValue()); + self::assertEquals('#REF!', $sheet->getCell('A2')->getCalculatedValue()); + self::assertEquals('#REF!', $sheet->getCell('A3')->getCalculatedValue()); + self::assertEquals('#NAME?', $sheet->getCell('A4')->getCalculatedValue()); + self::assertEquals('#REF!', $sheet->getCell('A5')->getCalculatedValue()); + } +} diff --git a/tests/data/Reader/XLSX/bug1686b.xlsx b/tests/data/Reader/XLSX/bug1686b.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..d496c5c76e8235f805e988117eaa75e1a39782f0 GIT binary patch literal 11740 zcmeHtWmH_-(rpKKcM0we!QF$qyEN|Z1b3I<0fHq+aF^f&f|tqpNL1Q>NL01R~h|E~XH1bR~B6u?PPLJuh)!Vonk3%!{8Ty{I- zl8E@Hh?u)-Wd^~FD(fGYwY2d~ZW8*~^;0XE4{w=`LOX}^t9l8@a1Uu6^)JK&C-e*-otZN0Llp$RB~;6or_q&>dfx|Ca`ueygRGAY@$n0J zAb>rHW-!X{QJMA-t$9kOJQN4%zya)#1M?F8p^Y@cLJFgyG^SjGf*8UYgPbzOx!Aw) z>Wk^{cU8dWLr~GTtIKO|IFmkid-fCWUsoR`5MV#{1o$}SgWp;#Mz3FL z&7@yf_!4us?+FeuN2B#I+*ai$5026j+S6xUrmgMd_D z`l35Z$Y$emdF`BpD^3)?VB53yto_Cke+iyGm<4yyWy&_&a&4geW`@d+scy2Q!DHy| z3PBJQsgIA~0L8z=aGe@6*%^qKvY-%!2gR^~6VS$)iSftl|6=#QIADK!^s+=bg>Ghq z(8HH^VFQ=bOYtZoGVa1JTS!#>UP&z?*TuXg$6xB8B0y2a3j&w)Z}YnwTv+0Z*%=_Y zSbb3zg@(aPR_|UO`ufJv6`GdPDOJL;>{}12+w}SLMY^Q4C$(E!9BpZ1NuJE76*7s* zW3eifQAQ0sSoA`IP%QrR0PQ|Gt!3k@if7XzD*NT3l?|M^J4vIN{xhjX+lYeU+_HNU z>6in~#^!SsJ_9x+=ht{@Di++9mBu+vJfxlmrZyi>#WFk4?|m5Lvj&t&U!YyFj!O1Z z;a-t^rP%t=TM5T05f2j=%MWDq?)HP9mO`Ok8g~ z;6V)kzWZocusAA1aB=252nwLaHs(z7J;0K~QeI{vYsy(Wz*PG*5k~VQeeN?ig;tUZ z?O<0=>P9++QOYt2#2rqw7Y~poX1A8Xl7a_C9-j2$9i{oHS&Vv7|HqVI!CGgs@*!@a z#hSnfD#-WNK)kHs_ZdmIV2k>s8ahr_o=0)?I7c^>DEoS9)yXq*{Fqb#TQ`Zr>o|J! zJZuupgZ{5v-#Nokf7>h(mZYyb^ge%3XFuIP1RA)HeL2JWCH}Q^!XccPuuh>^`WK!TxE4pY2 z)(lKgUKl>vEXS^4VWw}+9s?rCQx4L<8kmb--j{wbghT#f(}JNJbXapmK7qUpCaJbSOiq=lMXt_NhmdX!IjIYOVShfrd!-dxpx`|}R?abh)HdEn zyRj>?rF`7nn6EPU4L8@}czOAvk^%Wq;(+={G*{8MEI++Wv(ebR?xeC4Vq0{B%J0_h z3@JNUqlp*YmIndgSYQsK@@y zw_ehfetm^0+~175q;3Flf{MiCh*LGINw89j>>MuMdBC6u`9QR8Vc#Jb(NUH~t|~pE z-tk$v`abE);tzXQ{3_W+Vzpz4Kr+=`q1aDW?ls)|9O~p=<2Qk&nti^mH|~)$Uwwt+ zRORhrRig#;WEo?p_=yw_?9kQtpxgGVNhEtooU^;XDS$%y@0C%oI`=IVC>`8F004LZ zm}j7%|D!nm9{T?(l+QpS7j*A`_tlf!Dc!>i5_y4#icSrdr4^)^2g0aWXQaE}2-~%Y za!HBN&A#5E3H9(YZ5VA3W}$bF{j!t`OI6e3R6HNbQ0ZgPvGNk?6^qdJ?vH$2M@bb7 z;l{q?8uEM94>2*-+m{4%Q5cktNDB9QbXw&f1UNHU+jREVi=xvOx-uxj==Ji(BdQM6 zkk2`A&KO=tnAI{tINy?95#2CyJyzHbDNFfOpS%?pHK%WcU?8$qqzn%%jfcQjaU=%b z@%5Z%pm>nGK%GUg4)?H(dI`i$P*k_DfAWcW`QfD^pfmW(gJ~*PH9L2EnKe{ugpsVL zfNC{G+F6_t7FDg5tH{)DWx(3*xoGQD)B^iH%^th_f#^V(a}^}#W0Sy|%r`0n^#{a% zq8S7f$*K`Zd~HDj05~B2{8?L^Er385XQrQTEI*1&R`Nn@<_~b`&Ixrdc~HQ)NxEpa zDR=a3)NGM2V{y^wUcK8V$5->cjLjoSskR!I7RF2ZcDs2b>U61#wGK1?Qf-*#G&EU= zLY1;~Kg;j7`ME3&OR5GglPaXM7nJDXM1y@|(u8*ncqIgEO8R7Wo$KDM#9r=tp9;I` zl|qHfCx++eUpVWaa@dhmLWmH;Ogh%9^BG9K_~Oux7QXVPK%!8s53!(b?7bP5|MFdg zPMk>h)A@rUaxks6n2jmPep7(5a^#hf4$Uwp#kI>`?Lc(6=t|t@(j7DLoQ|bd6nI8eM&apJQbDq0@)AN4GsN z%<43mxd!ta#Cku{8-C?Ot zHDu{md;-9NA*T$JxxDR?=UaN9TiM0(Ezt-P;OXLyo<)LnlDS;PslP=%;thGx)Q4T? zYO*z&VH$rbYV>Amn`~fzB;RITe73|{IxG9I6Q-70+S^FulD~mZu9gRNkt5a8>R_1= z%j9k~$dXidC(!8L(+X?^O;91p1kVG9uu3EV8;qBhv4RuRxk|`l&Zyt-u~xgoJ3TgY zPp*G4R9)@st z$Joa?b?dS(NbDb_jQbSas`ypc0sb|-_VlvCoPlYq85J8GUNd|uZ3agxG$!v z%`^#dNz{nO_Aruufi%^We8Yt1{*aKHT<0aSQD-2ojMsO>uV-;O(mnOpDYi)|3d(ANL z+_X(FPruH`Hjcx6k$pWSG(nz@-doUTE@E_OlvbajTZXtomv@hA-20Jl%e{?YDWvF% zpR0GzPEca81QkVx`{g5tFpF}l$98R%4X@BLRR&9}JXSl#mpwGI8-H{uDZ3c9^0)xm zkqrAugLp`zJL%@(M@EF znipP!IhM)% zcs20&OekcqlAng>hNq%FV6o=|Z#(@t`#*@dwT1PKt|Gh$CB+_I(B?u~g3R@Bi)l8? znJ#3Pgb2xJ#w3HcRe-O2^sZv`F=cI#71eChXw%p zh<|MO{LED@7C>7d)6dtRX=+bvI0~N=qXYX!5W(5~j(shTV)^T+P0|v%X?8L}UHzVt zIvZRD*WK6aK8|*);OWRDWwO>K8r=FO#EDfv|Sh@bq^w3vc|K zP;EcK^v{A{i`nF*5=n6{&|>w%geEk8G~)_bp^WZ-*+upStDk@{XgLjwu28ughb-|* zKhRjw+n1GuY!5L-+TZ(KtHL*y1QEn&>J^RmRTOzuYu(i%;UQbf2ZSW^Cq_9eyw6v? zCoZYc(EKc}DegD1813k|?aboR%a3gwXs| z9R7najeu@;aRxJ85LXdD< zBF(5$JYwmdvWl!GDd&cE^Z}~?;1!rMxzy^XUIc|PnCiwTYVNN+3lx(uX!fCKl!lF} zG>Fn_S04ld9*W2!>zP>#QuGWXH1U-$JdArj$=2V|dsMOvf2K-rr*{8+1Gz?HqG%Pf;Cr^>o}_554YqJmxvCTwTOuYWF=k z*-pY<@jc&Eh$Ea|ARtUTB8jLvs`zGdNEu}mfOLEOOfb8LS)?aX2zAS?1(sNwgb|JI~km8AC@$4mq+*NvBN_EfHf0 z)(4EzWq`{~wp@)><++$j;CIAGw1^_E&Iy%{D$!`d(iYM1jTwH{RoTjjvjdb5LE)1W zqE3bs%8a$5#^FO!R#0QZliC=f>CKtnOls7ov=O1 zbe+}7zf+R3f{9$rE0X5)#)Ljt<~vW=Ii7F_9KssX?Q^{zjzk%!t%;^69JU00x7ip) zKIeJa#DN|^N+c5T1ot2*Y@fI}UNL@A0>x^U#Rk~=u6r}mM#y!mjAW|t8{i0GRcmy5 z8xgikU}Y3=p09qkZa9LCm*@0d%6tosic54Kj(3BAv*wr)+t6myxCW_V3gMn~8Qs8$ z+Ig1YGhpI->n}7;uVLtbm&eJrmFD)2fgV`Y5vVH#*LY$}(t2DQS*l*yu}E@X*}*o5 z)}tnAW+1ni6I(i@c4J_)%wKz99r)-u(D~Wd^cxy1^Y6Dq;QCnM5??f245KJn2l5$( z!oVS5F9n8UKIn>NmulM_BxlZMav+0aNT|1=C#}cyjz&~d-MJ4wcYo##1sSJve4EUZ zNe$c-3#fs;B%-(WvdCHd=#W_E{KZqvS%3QK@%Fvhq`trh;D~klsdnG_#ksImdaF;s zAm!o7R*phz)dm|$^1%>fZ;Ob#FGTBjG&FO`f>JY8lhv~k(yI*iH2TG4GKMurN9KL~RlIE@On6>6)Qli`(*(1)HoY~v0&XN-!YhgGUHUUcV+DT>^x`d;YBTp3YO{K!&;eGE4 zVHx=xL0215Ok5i{&n72ofu<%C9dvBOtEeESone-M%u39JjcfN+!$FIg*2oF&{I0|zaoq4;Ad};5&FWMBAIEkhQouMqhn-}%c?o7EPZHMu$ zBbJ_sPF2$u+TF`DI8|6PHX)%+drG%{oAQkU?__3p^O;pnt5v=`vZh99p_Yvv20>wp zYEAtfn*ND zsKq3$7?_n-ZYqXHKN=h8CFxm)WIUq|6xAT*JkCw$KlINq9;|IJ42PgbMadd`#;*Hzv)ZmZ=i_*YN}PRLH<+ts>t+>VR>w~b z(I)?PZ0`$Ch<>@wA)?n(jY@2yq|bc!@@Ndx|U z?maP`iQxu~uW38r(yFEY5v08SUVc6EUfC3Y3bQh3-U8|+ezXeCE}k|(=O03>K+V<~ zB*Y#BX8loa4hG%U=wa)JCZ-g(`f>^PN{xpiO7x@2%}4qkZspzd33%$q{UYQyC!I%^ zoRuds<>6{g`kcyH3G_m6Sk!hROXWg41TZ@h$aYKKmyAUMJx1xZVCZrsE@~H@y}lPR!V> zhm~f)-?_?wKe*t>lBm{Qv`^-6VR&VQEoTwYY++?Qm2Q%_d`lX)UO28ka{%WMQDLoL z{n};IRNlf<0&IPTL>v}&Eoz?kRJs6`#iqZ_hfBe_JWIULE(M$=pv1m&Rcrh3{n49N z=kP(5Tk!o7fI_fTPO7#=5W!36>|OpyX%4YP<<6PV+sX`S#)}}{i&*`A(X90`??dvO zexoXfaCk>!OmDES11j;OEN|gvbA#?{?TOn0rSJ`J8vVuW5tbw!S$TwRf<2qR6a{_~ zxLn&BROz&W7DHUIx!-XC@37t{D0x$iS&Uy^a#V;wOa(qjla3PSchfPAJ*yX^N(aAv zP>RsaeWd^~|8t&o*pSw2b({yXd)2N2j;$`h#m#VQKuN_^?Y(4YnK0jP3 zr8#2xVkp?`rEi!?#d|j*mLJM{gX!~Xd%Lmgc6;m(&n8uUfB@hXuYM| zrk<}cK81Uq44RXYE4|?3%JHdo&}HqMRw{ifV!{*-vCx4b94j4v`bPKx|DWir6s5$+ z03tdah|;8g&>86A0(7!7vXQm4vvy|s{nJlz8J*Ax+Wq^{Y_y9G=+)>HOaa|PjNwJL zdoP~#?UmunvJR0)w)?&GttU*{jpZUlS=fDxIQpIB8+~nKXqLKG^*r-n zN|*3)t@A|ShrxhjCt?hFK`_UUnYMxx>t5kzz_N%ibQ!qfsNQ+>?m^0`TOTKq;uEC( zXfwE}K1UH6E1kE;`rlOLpkvObPkG1ZoIB%So{u?C=!6L%)#KWtDG0a}AuBr=Wk{do ze*)^|9GoH|b;~ih8PYZJ<=By`>Ai1x$1SF5%y&q@>zJWM?lhQE1w>eaW}}$C7F=sC zX3TqzSVUyUR`Cde^6$B$WY-Uc5d=2v7jBR4@64xb6 zK7{3bYf?sLPgR!T18@sh4V|ZFtIX$9b%e{H2M$cgWSmge>To@grBhqvA7H}=Pxcz` zM@;_!`|eyseuHoxcNn82W-T~zs$OEPt?OMD65ej1ClR+4ck0< zj?b1Xkq`)MIFIpl#n;8RK6GK0cpLN>~oD=;c+VKFy$|4ZjOCYrW92nRFU5rePT#SC?YEvZ{ z1$-8ic8yg*&35}%5)6^r7HY5!F$z`co$0Fn2(=DVZ%cKxM{o1CU?{G+HC+Ew{*25Z z_H{_(iRC&Mfl``hRIO8XGZRD#nO0I#gNssjtwE_s@mq547ll>vP3!6C5DxDOZ?n21 ztH>`5Ju|KWNW6oRLu{#z{mS7Vf%D*|*0Y{^2CPGMZ_&GUh_9(IO2yAUmGYr4vwld% zI)XL#>s|lKHFAP;E!4tkry%-DEcj4Q!j=dRq9Zr=&87Qp*kQ8f{_|TDpU=c^R_=sS z*|kZ1a3mnmJO(Q{zWX14mP0Nf%EVelLOo6m9QS-{LhCJc)?we~xoFKk-35rt{FK3J z@Z7Gg?(0c#ugXLVOM)XvNPmN4{*I3)*j`g%)Q=K5R-xp3Ko$*7k$wrLU^Dx$gU7Xg zn>-|SQ=)TYPS>zd7@x1~gEZZHPNC{^eg0lMJVbjcU!Vgks>6a5s?Lkuhac|&g=H^| zUAz3Ch#1a3Bm)r{)>LL!1lyf-d(Dhv!tjvWeOSp@=_tS%!<6-CBV3bMz9TDb;vTf+ z@%I2~5K=s=11&ZiXjxJI$W|ssHYTDLMwWIz*3>6X53Y+DC3pq$uyU_Ou3Rwu1p}!E z7{Aa<4fFv8slcuMnEc+rtLjR&&wCB7-ZfAa82J~mYpv9%15zuN?%zCko}1wZrv>i@ zAIEYn(-<$vQq^Hd9HfjPIR!-#s7c?aR#9VWp~9~u4UNn}(<$qBhx7`>Z{?Fpf2n*= zKlAxJn2i-|z?xl1E*p5Dr~F02Yckv(dp2|*Al4&m5ci1c;b#Y~q*jmWkD>0FpdS7o z9&uh(M@$m*c$Og6qJnn3P3=t-o$MW)nM~}RfIprQG!g#aMmGq$=tL!jZeom(6}dZP z#6yO{;YD)W*Clh4SaMn8SD_9K>{!x({fooGef84QPD!p!zbt1_j+G(WD+rSz?Tv6T z7C50}?PROWx5ekAS$Q9MiXcj@Ah~&7iJz{XkgjE%%zq4e$Bv^Bh{z|UHRu}aHewlB zwZR+zSzwPH#ttLtAc~%%X^l+Nf`*=y%oH0&Z&E%5^QP zuOej~1`(MxGFRK5VbbUMB*_9j$ZKWF(p>2}4&`81zaI^GS9sfLybTM2@nN2y;i4Jc z-e2+xh$NR^jVCwCs#t@~mZeE=s~>$#vk$t!KEYY_7fV!jlAGKP_W;Ad6}Tnn#r{Y3 zWp5P-z5}tZ1QdQKzq8NC!Qp@813~uZm8BwQzrc*phCJ)f>|(!OAkG3&9Z9-B)-4c7 zu|JX}+b?4K2}O%+?!y~`@`_1J@Ou1SNqzwVr5&pk4z0L=GWU?QQKy7Fw)R>(jquz3 z{u*RbGAIp^HA1Y_SL79Ex9(PiK4Ae2%2+f6nE7T0smh7(svtAOXT=Ln@*rdTE0fya zmv!RX=V-goV|)71IF4>f(~mYMD3bR9M;~Rhz1X(ihVAD?1=ckmH22|sS>gJFT%@ zC<=IixQO&v*n78+71Z~_de3*K>x)y+S)I1%S58^(HyKXJsq|8`oI#N9Htpdh*kTnk zuGq!kxtg5w4^nbc?0Fr+sJ){Fv@Ty>E`3=6tew)mp`0!XeFfBwc7*Q>ZjM4G$Otf1 zLw-mdW%mc`mKP|9E8cuvbH^9A%khck`lR=a3A9juuOC&Oeznt}UwAUyLlm# zv5Yt2OC_XiK1L*)m(TG;4(Kr^XRvMJDP@#JFG z`nweU>fC-|1G(oNAO!x7k2jyX}vwqR`Tmp%uR+p?U){f=)o%g7RLU$*Y7T)E?gk|tX(Fbvl&Z$HXJ z9PV1pvmOk`__C6ah$vJ6AwwrTi-{bMA_1&lSHF;ajoE6)sdFleW~QX6%{5g#vnEHW z;?|FB*8nP-Q?{xBZfChdD2uU>eWmIxC%Z~cBOCW1pWO1DWQxozLp1yL(ZS@kXpQy6 zA-6|;y~#LQ=u<+hH{x(*>^&j`3^6_Udth|DQw8z!(7k?#LzRroUYOZ~#+L z=C2O^dfw}I>mE?v{q6MEQ^TicV1AiygSI37oLc_tM9fp;zs}NsnF0XIa6gUz&1C(l zpQpppU!KH48zTR1bo$iG)7jQ9FN+|t{tsS$678v%r^Aq6UXD?JdU-k;dFtTlcFr#c z4!A!Z{My`kYWlQw{$(mi@YD47H1ZFP^izLNyTf1p$_W1tfB%Th)Bfj|KRi&w1VZ@t zZs_Tsd8)L38SfMSH2#b3ero+!5&O#)02m_y0RARvpPK(wh5T+VPyU Date: Thu, 10 Dec 2020 18:13:12 +0100 Subject: [PATCH 09/30] Update change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ba28c26..c818cb93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Resolve issues with defined names where worksheet doesn't exist (#1686)[https://github.com/PHPOffice/PhpSpreadsheet/issues/1686] and [#1723](https://github.com/PHPOffice/PhpSpreadsheet/issues/1723) - [PR #1742](https://github.com/PHPOffice/PhpSpreadsheet/pull/1742) - Fix for issue [#1735](https://github.com/PHPOffice/PhpSpreadsheet/issues/1735) Incorrect activeSheetIndex after RemoveSheetByIndex - [PR #1743](https://github.com/PHPOffice/PhpSpreadsheet/pull/1743) - Ensure that the list of shared formulae is maintained when an xlsx file is chunked with readFilter[Issue #169](https://github.com/PHPOffice/PhpSpreadsheet/issues/1669). - Fix for notice during accessing "cached magnification factor" offset [#1354](https://github.com/PHPOffice/PhpSpreadsheet/pull/1354) From a8462f386455cba3daba731174de3c8cb2c29e82 Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 10 Dec 2020 09:19:56 -0800 Subject: [PATCH 10/30] 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. --- docs/topics/accessing-cells.md | 6 +- src/PhpSpreadsheet/Style/Style.php | 30 ++-- .../Worksheet/ColumnCellIterator.php | 13 +- .../Worksheet/RowCellIterator.php | 14 +- tests/PhpSpreadsheetTests/Style/StyleTest.php | 138 ++++++++++++++++++ .../Worksheet/ColumnCellIterator2Test.php | 75 ++++++++++ .../Worksheet/ColumnCellIteratorTest.php | 12 +- .../Worksheet/RowCellIterator2Test.php | 75 ++++++++++ .../Worksheet/RowCellIteratorTest.php | 15 +- 9 files changed, 342 insertions(+), 36 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Worksheet/ColumnCellIterator2Test.php create mode 100644 tests/PhpSpreadsheetTests/Worksheet/RowCellIterator2Test.php diff --git a/docs/topics/accessing-cells.md b/docs/topics/accessing-cells.md index edb71514..a777afc1 100644 --- a/docs/topics/accessing-cells.md +++ b/docs/topics/accessing-cells.md @@ -422,8 +422,10 @@ foreach ($worksheet->getRowIterator() as $row) { $cellIterator = $row->getCellIterator(); $cellIterator->setIterateOnlyExistingCells(FALSE); // This loops through all cells, // even if a cell value is not set. - // By default, only cells that have a value - // set will be iterated. + // For 'TRUE', we loop through cells + // only when their value is set. + // If this method is not called, + // the default value is 'false'. foreach ($cellIterator as $cell) { echo '' . $cell->getValue() . diff --git a/src/PhpSpreadsheet/Style/Style.php b/src/PhpSpreadsheet/Style/Style.php index c1aa319e..f7c1be23 100644 --- a/src/PhpSpreadsheet/Style/Style.php +++ b/src/PhpSpreadsheet/Style/Style.php @@ -42,13 +42,6 @@ class Style extends Supervisor */ protected $numberFormat; - /** - * Conditional styles. - * - * @var Conditional[] - */ - protected $conditionalStyles; - /** * Protection. * @@ -85,7 +78,6 @@ class Style extends Supervisor parent::__construct($isSupervisor); // Initialise values - $this->conditionalStyles = []; $this->font = new Font($isSupervisor, $isConditional); $this->fill = new Fill($isSupervisor, $isConditional); $this->borders = new Borders($isSupervisor, $isConditional); @@ -212,6 +204,8 @@ class Style extends Supervisor $rangeEnd = Coordinate::coordinateFromString($rangeB); // Translate column into index + $rangeStart0 = $rangeStart[0]; + $rangeEnd0 = $rangeEnd[0]; $rangeStart[0] = Coordinate::columnIndexFromString($rangeStart[0]); $rangeEnd[0] = Coordinate::columnIndexFromString($rangeEnd[0]); @@ -361,6 +355,13 @@ class Style extends Supervisor for ($col = $rangeStart[0]; $col <= $rangeEnd[0]; ++$col) { $oldXfIndexes[$this->getActiveSheet()->getColumnDimensionByColumn($col)->getXfIndex()] = true; } + foreach ($this->getActiveSheet()->getColumnIterator($rangeStart0, $rangeEnd0) as $columnIterator) { + $cellIterator = $columnIterator->getCellIterator(); + $cellIterator->setIterateOnlyExistingCells(true); + foreach ($cellIterator as $columnCell) { + $columnCell->getStyle()->applyFromArray($pStyles); + } + } break; case 'ROW': @@ -372,6 +373,13 @@ class Style extends Supervisor $oldXfIndexes[$this->getActiveSheet()->getRowDimension($row)->getXfIndex()] = true; } } + foreach ($this->getActiveSheet()->getRowIterator((int) $rangeStart[1], (int) $rangeEnd[1]) as $rowIterator) { + $cellIterator = $rowIterator->getCellIterator(); + $cellIterator->setIterateOnlyExistingCells(true); + foreach ($cellIterator as $rowCell) { + $rowCell->getStyle()->applyFromArray($pStyles); + } + } break; case 'CELL': @@ -599,18 +607,12 @@ class Style extends Supervisor */ public function getHashCode() { - $hashConditionals = ''; - foreach ($this->conditionalStyles as $conditional) { - $hashConditionals .= $conditional->getHashCode(); - } - return md5( $this->fill->getHashCode() . $this->font->getHashCode() . $this->borders->getHashCode() . $this->alignment->getHashCode() . $this->numberFormat->getHashCode() . - $hashConditionals . $this->protection->getHashCode() . ($this->quotePrefix ? 't' : 'f') . __CLASS__ diff --git a/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php b/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php index 12420d77..714ee7ce 100644 --- a/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php @@ -92,10 +92,11 @@ class ColumnCellIterator extends CellIterator */ public function seek($row = 1) { + if ($this->onlyExistingCells && !($this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $row))) { + throw new PhpSpreadsheetException('In "IterateOnlyExistingCells" mode and Cell does not exist'); + } if (($row < $this->startRow) || ($row > $this->endRow)) { throw new PhpSpreadsheetException("Row $row is out of range ({$this->startRow} - {$this->endRow})"); - } elseif ($this->onlyExistingCells && !($this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $row))) { - throw new PhpSpreadsheetException('In "IterateOnlyExistingCells" mode and Cell does not exist'); } $this->currentRow = $row; @@ -113,7 +114,7 @@ class ColumnCellIterator extends CellIterator /** * Return the current cell in this worksheet column. * - * @return null|\PhpOffice\PhpSpreadsheet\Cell\Cell + * @return \PhpOffice\PhpSpreadsheet\Cell\Cell */ public function current() { @@ -180,18 +181,12 @@ class ColumnCellIterator extends CellIterator ) { ++$this->startRow; } - if ($this->startRow > $this->endRow) { - throw new PhpSpreadsheetException('No cells exist within the specified range'); - } while ( (!$this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $this->endRow)) && ($this->endRow >= $this->startRow) ) { --$this->endRow; } - if ($this->endRow < $this->startRow) { - throw new PhpSpreadsheetException('No cells exist within the specified range'); - } } } } diff --git a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php index f5576dc7..9b9d54eb 100644 --- a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php @@ -93,12 +93,14 @@ class RowCellIterator extends CellIterator */ public function seek($column = 'A') { + $columnx = $column; $column = Coordinate::columnIndexFromString($column); - if (($column < $this->startColumnIndex) || ($column > $this->endColumnIndex)) { - throw new PhpSpreadsheetException("Column $column is out of range ({$this->startColumnIndex} - {$this->endColumnIndex})"); - } elseif ($this->onlyExistingCells && !($this->worksheet->cellExistsByColumnAndRow($column, $this->rowIndex))) { + if ($this->onlyExistingCells && !($this->worksheet->cellExistsByColumnAndRow($column, $this->rowIndex))) { throw new PhpSpreadsheetException('In "IterateOnlyExistingCells" mode and Cell does not exist'); } + if (($column < $this->startColumnIndex) || ($column > $this->endColumnIndex)) { + throw new PhpSpreadsheetException("Column $columnx is out of range ({$this->startColumnIndex} - {$this->endColumnIndex})"); + } $this->currentColumnIndex = $column; return $this; @@ -181,15 +183,9 @@ class RowCellIterator extends CellIterator while ((!$this->worksheet->cellExistsByColumnAndRow($this->startColumnIndex, $this->rowIndex)) && ($this->startColumnIndex <= $this->endColumnIndex)) { ++$this->startColumnIndex; } - if ($this->startColumnIndex > $this->endColumnIndex) { - throw new PhpSpreadsheetException('No cells exist within the specified range'); - } while ((!$this->worksheet->cellExistsByColumnAndRow($this->endColumnIndex, $this->rowIndex)) && ($this->endColumnIndex >= $this->startColumnIndex)) { --$this->endColumnIndex; } - if ($this->endColumnIndex < $this->startColumnIndex) { - throw new PhpSpreadsheetException('No cells exist within the specified range'); - } } } } diff --git a/tests/PhpSpreadsheetTests/Style/StyleTest.php b/tests/PhpSpreadsheetTests/Style/StyleTest.php index b695a50b..6f157709 100644 --- a/tests/PhpSpreadsheetTests/Style/StyleTest.php +++ b/tests/PhpSpreadsheetTests/Style/StyleTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Style; use PhpOffice\PhpSpreadsheet\Spreadsheet; +use PhpOffice\PhpSpreadsheet\Style\Fill; use PHPUnit\Framework\TestCase; class StyleTest extends TestCase @@ -19,4 +20,141 @@ class StyleTest extends TestCase $outArray = $cell1style->getStyleArray($styleArray); self::assertEquals($styleArray, $outArray['quotePrefix']); } + + public function testStyleColumn(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $cellCoordinates = 'A:B'; + $styleArray = [ + 'font' => [ + 'bold' => true, + ], + ]; + $sheet->getStyle($cellCoordinates)->applyFromArray($styleArray); + $sheet->setCellValue('A1', 'xxxa1'); + $sheet->setCellValue('A2', 'xxxa2'); + $sheet->setCellValue('A3', 'xxxa3'); + $sheet->setCellValue('B1', 'xxxa1'); + $sheet->setCellValue('B2', 'xxxa2'); + $sheet->setCellValue('B3', 'xxxa3'); + $sheet->setCellValue('C1', 'xxxc1'); + $sheet->setCellValue('C2', 'xxxc2'); + $sheet->setCellValue('C3', 'xxxc3'); + $styleArray = [ + 'font' => [ + 'italic' => true, + ], + ]; + $sheet->getStyle($cellCoordinates)->applyFromArray($styleArray); + self::assertTrue($sheet->getStyle('A1')->getFont()->getBold()); + self::assertTrue($sheet->getStyle('B2')->getFont()->getBold()); + self::assertFalse($sheet->getStyle('C3')->getFont()->getBold()); + self::assertTrue($sheet->getStyle('A1')->getFont()->getItalic()); + self::assertTrue($sheet->getStyle('B2')->getFont()->getItalic()); + self::assertFalse($sheet->getStyle('C3')->getFont()->getItalic()); + } + + public function testStyleRow(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $cellCoordinates = '2:3'; + $styleArray = [ + 'font' => [ + 'bold' => true, + ], + ]; + $sheet->getStyle($cellCoordinates)->applyFromArray($styleArray); + $sheet->setCellValue('A1', 'xxxa1'); + $sheet->setCellValue('A2', 'xxxa2'); + $sheet->setCellValue('A3', 'xxxa3'); + $sheet->setCellValue('B1', 'xxxa1'); + $sheet->setCellValue('B2', 'xxxa2'); + $sheet->setCellValue('B3', 'xxxa3'); + $sheet->setCellValue('C1', 'xxxc1'); + $sheet->setCellValue('C2', 'xxxc2'); + $sheet->setCellValue('C3', 'xxxc3'); + $styleArray = [ + 'font' => [ + 'italic' => true, + ], + ]; + $sheet->getStyle($cellCoordinates)->applyFromArray($styleArray); + self::assertFalse($sheet->getStyle('A1')->getFont()->getBold()); + self::assertTrue($sheet->getStyle('B2')->getFont()->getBold()); + self::assertTrue($sheet->getStyle('C3')->getFont()->getBold()); + self::assertFalse($sheet->getStyle('A1')->getFont()->getItalic()); + self::assertTrue($sheet->getStyle('B2')->getFont()->getItalic()); + self::assertTrue($sheet->getStyle('C3')->getFont()->getItalic()); + } + + public function testIssue1712A(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $rgb = '4467b8'; + $sheet->fromArray(['OK', 'KO']); + $spreadsheet->getActiveSheet() + ->getStyle('A1') + ->getFill() + ->setFillType(Fill::FILL_SOLID) + ->getStartColor() + ->setRGB($rgb); + $spreadsheet->getActiveSheet() + ->getStyle('B') + ->getFill() + ->setFillType(Fill::FILL_SOLID) + ->getStartColor() + ->setRGB($rgb); + self::assertEquals($rgb, $sheet->getCell('A1')->getStyle()->getFill()->getStartColor()->getRGB()); + self::assertEquals($rgb, $sheet->getCell('B1')->getStyle()->getFill()->getStartColor()->getRGB()); + } + + public function testIssue1712B(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $rgb = '4467b8'; + $spreadsheet->getActiveSheet() + ->getStyle('A1') + ->getFill() + ->setFillType(Fill::FILL_SOLID) + ->getStartColor() + ->setRGB($rgb); + $spreadsheet->getActiveSheet() + ->getStyle('B') + ->getFill() + ->setFillType(Fill::FILL_SOLID) + ->getStartColor() + ->setRGB($rgb); + $sheet->fromArray(['OK', 'KO']); + self::assertEquals($rgb, $sheet->getCell('A1')->getStyle()->getFill()->getStartColor()->getRGB()); + self::assertEquals($rgb, $sheet->getCell('B1')->getStyle()->getFill()->getStartColor()->getRGB()); + } + + public function testStyleLoopUpwards(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $cellCoordinates = 'C5:A3'; + $styleArray = [ + 'font' => [ + 'bold' => true, + ], + ]; + $sheet->getStyle($cellCoordinates)->applyFromArray($styleArray); + $sheet->setCellValue('A1', 'xxxa1'); + $sheet->setCellValue('A2', 'xxxa2'); + $sheet->setCellValue('A3', 'xxxa3'); + $sheet->setCellValue('B1', 'xxxa1'); + $sheet->setCellValue('B2', 'xxxa2'); + $sheet->setCellValue('B3', 'xxxa3'); + $sheet->setCellValue('C1', 'xxxc1'); + $sheet->setCellValue('C2', 'xxxc2'); + $sheet->setCellValue('C3', 'xxxc3'); + self::assertFalse($sheet->getStyle('A1')->getFont()->getBold()); + self::assertFalse($sheet->getStyle('B2')->getFont()->getBold()); + self::assertTrue($sheet->getStyle('C3')->getFont()->getBold()); + } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIterator2Test.php b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIterator2Test.php new file mode 100644 index 00000000..c542d89e --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIterator2Test.php @@ -0,0 +1,75 @@ +getActiveSheet(); + $sheet->getCell('B2')->setValue('cellb2'); + $sheet->getCell('B6')->setValue('cellb6'); + + $iterator = new ColumnCellIterator($sheet, 'B', 1, 8); + if (isset($existing)) { + $iterator->setIterateOnlyExistingCells($existing); + } + $lastCoordinate = ''; + $firstCoordinate = ''; + foreach ($iterator as $cell) { + $lastCoordinate = $cell->getCoordinate(); + if (!$firstCoordinate) { + $firstCoordinate = $lastCoordinate; + } + } + self::assertEquals($expectedResultFirst, $firstCoordinate); + self::assertEquals($expectedResultLast, $lastCoordinate); + } + + public function providerExistingCell(): array + { + return [ + [null, 'B1', 'B8'], + [false, 'B1', 'B8'], + [true, 'B2', 'B6'], + ]; + } + + /** + * @dataProvider providerEmptyColumn + */ + public function testEmptyColumn(?bool $existing, int $expectedResult): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('B2')->setValue('cellb2'); + $sheet->getCell('B6')->setValue('cellb6'); + + $iterator = new ColumnCellIterator($sheet, 'C'); + if (isset($existing)) { + $iterator->setIterateOnlyExistingCells($existing); + } + $numCells = 0; + foreach ($iterator as $cell) { + ++$numCells; + } + self::assertEquals($expectedResult, $numCells); + } + + public function providerEmptyColumn(): array + { + return [ + [null, 6], + [false, 6], + [true, 0], + ]; + } +} diff --git a/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php index 2083f347..1fa25330 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php @@ -71,11 +71,21 @@ class ColumnCellIteratorTest extends TestCase public function testSeekOutOfRange(): void { $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); - + $this->expectExceptionMessage('Row 1 is out of range'); $iterator = new ColumnCellIterator($this->mockWorksheet, 'A', 2, 4); $iterator->seek(1); } + public function testSeekNotExisting(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $this->expectExceptionMessage('Cell does not exist'); + + $iterator = new ColumnCellIterator($this->mockWorksheet, 'A', 2, 4); + $iterator->setIterateOnlyExistingCells(true); + $iterator->seek(2); + } + public function testPrevOutOfRange(): void { $iterator = new ColumnCellIterator($this->mockWorksheet, 'A', 2, 4); diff --git a/tests/PhpSpreadsheetTests/Worksheet/RowCellIterator2Test.php b/tests/PhpSpreadsheetTests/Worksheet/RowCellIterator2Test.php new file mode 100644 index 00000000..20d10da9 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/RowCellIterator2Test.php @@ -0,0 +1,75 @@ +getActiveSheet(); + $sheet->getCell('C2')->setValue('cellb2'); + $sheet->getCell('F2')->setValue('cellf2'); + + $iterator = new RowCellIterator($sheet, 2, 'B', 'H'); + if (isset($existing)) { + $iterator->setIterateOnlyExistingCells($existing); + } + $lastCoordinate = ''; + $firstCoordinate = ''; + foreach ($iterator as $cell) { + $lastCoordinate = $cell->getCoordinate(); + if (!$firstCoordinate) { + $firstCoordinate = $lastCoordinate; + } + } + self::assertEquals($expectedResultFirst, $firstCoordinate); + self::assertEquals($expectedResultLast, $lastCoordinate); + } + + public function providerExistingCell(): array + { + return [ + [null, 'B2', 'H2'], + [false, 'B2', 'H2'], + [true, 'C2', 'F2'], + ]; + } + + /** + * @dataProvider providerEmptyRow + */ + public function testEmptyRow(?bool $existing, int $expectedResult): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('B2')->setValue('cellb2'); + $sheet->getCell('F2')->setValue('cellf2'); + + $iterator = new RowCellIterator($sheet, '3'); + if (isset($existing)) { + $iterator->setIterateOnlyExistingCells($existing); + } + $numCells = 0; + foreach ($iterator as $cell) { + ++$numCells; + } + self::assertEquals($expectedResult, $numCells); + } + + public function providerEmptyRow(): array + { + return [ + [null, 6], + [false, 6], + [true, 0], + ]; + } +} diff --git a/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php index bc2c16dc..4105c91c 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php @@ -73,9 +73,22 @@ class RowCellIteratorTest extends TestCase public function testSeekOutOfRange(): void { $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $this->expectExceptionMessage('Column A is out of range'); $iterator = new RowCellIterator($this->mockWorksheet, 2, 'B', 'D'); - $iterator->seek(1); + self::assertFalse($iterator->getIterateOnlyExistingCells()); + self::assertEquals(2, $iterator->getCurrentColumnIndex()); + $iterator->seek('A'); + } + + public function testSeekNotExisting(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $this->expectExceptionMessage('Cell does not exist'); + + $iterator = new RowCellIterator($this->mockWorksheet, 2, 'B', 'D'); + $iterator->setIterateOnlyExistingCells(true); + $iterator->seek('B'); } public function testPrevOutOfRange(): void From 4c8617551dc7927deca7e0c7148147daf0775a5a Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 10 Dec 2020 18:20:18 +0100 Subject: [PATCH 11/30] Update change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c818cb93..f07a84d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Apply Column and Row Styles to Existing Cells [#1712](https://github.com/PHPOffice/PhpSpreadsheet/issues/1712) [PR #1721](https://github.com/PHPOffice/PhpSpreadsheet/pull/1721) - Resolve issues with defined names where worksheet doesn't exist (#1686)[https://github.com/PHPOffice/PhpSpreadsheet/issues/1686] and [#1723](https://github.com/PHPOffice/PhpSpreadsheet/issues/1723) - [PR #1742](https://github.com/PHPOffice/PhpSpreadsheet/pull/1742) - Fix for issue [#1735](https://github.com/PHPOffice/PhpSpreadsheet/issues/1735) Incorrect activeSheetIndex after RemoveSheetByIndex - [PR #1743](https://github.com/PHPOffice/PhpSpreadsheet/pull/1743) - Ensure that the list of shared formulae is maintained when an xlsx file is chunked with readFilter[Issue #169](https://github.com/PHPOffice/PhpSpreadsheet/issues/1669). From 957cb62dab10cd347997d15f83056ee7671d18a8 Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 10 Dec 2020 09:35:26 -0800 Subject: [PATCH 12/30] 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. --- src/PhpSpreadsheet/Calculation/TextData.php | 17 +++++------ tests/data/Calculation/TextData/CHAR.php | 28 ++++++++++++++---- tests/data/Calculation/TextData/DOLLAR.php | 18 ++++++++++-- tests/data/Calculation/TextData/FIXED.php | 32 +++++++++++++++++++-- tests/data/Calculation/TextData/SEARCH.php | 5 ++++ 5 files changed, 82 insertions(+), 18 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/TextData.php b/src/PhpSpreadsheet/Calculation/TextData.php index da958836..f8974402 100644 --- a/src/PhpSpreadsheet/Calculation/TextData.php +++ b/src/PhpSpreadsheet/Calculation/TextData.php @@ -27,15 +27,15 @@ class TextData { $character = Functions::flattenSingleValue($character); - if ((!is_numeric($character)) || ($character < 0)) { + if (!is_numeric($character)) { + return Functions::VALUE(); + } + $character = (int) $character; + if ($character < 1 || $character > 255) { return Functions::VALUE(); } - if (function_exists('iconv')) { - return iconv('UCS-4LE', 'UTF-8', pack('V', $character)); - } - - return mb_convert_encoding('&#' . (int) $character . ';', 'UTF-8', 'HTML-ENTITIES'); + return iconv('UCS-4LE', 'UTF-8', pack('V', $character)); } /** @@ -160,7 +160,7 @@ class TextData // Validate parameters if (!is_numeric($value) || !is_numeric($decimals)) { - return Functions::NAN(); + return Functions::VALUE(); } $decimals = floor($decimals); @@ -174,6 +174,7 @@ class TextData } $value = MathTrig::MROUND($value, $round); } + $mask = "$mask;($mask)"; return NumberFormat::toFormattedString($value, $mask); } @@ -265,7 +266,7 @@ class TextData // Validate parameters if (!is_numeric($value) || !is_numeric($decimals)) { - return Functions::NAN(); + return Functions::VALUE(); } $decimals = (int) floor($decimals); diff --git a/tests/data/Calculation/TextData/CHAR.php b/tests/data/Calculation/TextData/CHAR.php index 1751699c..276d7015 100644 --- a/tests/data/Calculation/TextData/CHAR.php +++ b/tests/data/Calculation/TextData/CHAR.php @@ -9,6 +9,10 @@ return [ '#VALUE!', -5, ], + [ + '#VALUE!', + 0, + ], [ 'A', 65, @@ -22,27 +26,39 @@ return [ 126, ], [ - '⽇', + 'Á', + 193, + ], + [ + 'ÿ', + 255, + ], + [ + '#VALUE!', + 256, + ], + [ + '#VALUE!', // '⽇', 12103, ], [ - 'œ', + '#VALUE!', // 'œ', 0x153, ], [ - 'ƒ', + '#VALUE!', // 'ƒ', 0x192, ], [ - '℅', + '#VALUE!', // '℅', 0x2105, ], [ - '∑', + '#VALUE!', // '∑', 0x2211, ], [ - '†', + '#VALUE!', // '†', 0x2020, ], ]; diff --git a/tests/data/Calculation/TextData/DOLLAR.php b/tests/data/Calculation/TextData/DOLLAR.php index 95555e7c..67f394bb 100644 --- a/tests/data/Calculation/TextData/DOLLAR.php +++ b/tests/data/Calculation/TextData/DOLLAR.php @@ -6,11 +6,20 @@ return [ 123.456, 2, ], + [ + '$123.46', + 123.456, + ], [ '$123.32', 123.321, 2, ], + [ + '($123.32)', + -123.321, + 2, + ], [ '$1,235,000', 1234567, @@ -22,12 +31,17 @@ return [ -5, ], [ - '#NUM!', + '($1,200,000)', + -1234567, + -5, + ], + [ + '#VALUE!', 'ABC', 2, ], [ - '#NUM!', + '#VALUE!', 123.456, 'ABC', ], diff --git a/tests/data/Calculation/TextData/FIXED.php b/tests/data/Calculation/TextData/FIXED.php index b8bb36ca..2083ecce 100644 --- a/tests/data/Calculation/TextData/FIXED.php +++ b/tests/data/Calculation/TextData/FIXED.php @@ -20,13 +20,41 @@ return [ true, ], [ - '#NUM!', + '-123456.79', + -123456.789, + 2, + true, + ], + [ + '123500', + 123456.789, + -2, + true, + ], + [ + '123,500', + 123456.789, + -2, + ], + [ + '-123500', + -123456.789, + -2, + true, + ], + [ + '-123,500', + -123456.789, + -2, + ], + [ + '#VALUE!', 'ABC', 2, null, ], [ - '#NUM!', + '#VALUE!', 123.456, 'ABC', null, diff --git a/tests/data/Calculation/TextData/SEARCH.php b/tests/data/Calculation/TextData/SEARCH.php index 75aa7cea..28cd98f8 100644 --- a/tests/data/Calculation/TextData/SEARCH.php +++ b/tests/data/Calculation/TextData/SEARCH.php @@ -54,6 +54,11 @@ return [ 'Mark Baker', 2, ], + [ + 1, + '', + 'Mark Baker', + ], [ '#VALUE!', 'BITE', From 9289ab11b2484d8302b5317e12684855d89afd58 Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Thu, 10 Dec 2020 21:03:54 +0100 Subject: [PATCH 13/30] 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 --- composer.json | 2 +- composer.lock | 845 ++++++------------ src/PhpSpreadsheet/Writer/Html.php | 8 +- .../Writer/Html/XssVulnerabilityTest.php | 49 +- 4 files changed, 340 insertions(+), 564 deletions(-) diff --git a/composer.json b/composer.json index 7e3b840e..c6f8e30e 100644 --- a/composer.json +++ b/composer.json @@ -59,7 +59,7 @@ "psr/simple-cache": "^1.0", "psr/http-client": "^1.0", "psr/http-factory": "^1.0", - "voku/anti-xss": "^4.1" + "ezyang/htmlpurifier": "^4.13" }, "require-dev": { "dompdf/dompdf": "^0.8.5", diff --git a/composer.lock b/composer.lock index 5dd74833..a1fb99b7 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,62 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "209605c0b9329968170279f40db65d22", + "content-hash": "458fe4e974b469230da589a8781d1e0e", "packages": [ + { + "name": "ezyang/htmlpurifier", + "version": "v4.13.0", + "source": { + "type": "git", + "url": "https://github.com/ezyang/htmlpurifier.git", + "reference": "08e27c97e4c6ed02f37c5b2b20488046c8d90d75" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/ezyang/htmlpurifier/zipball/08e27c97e4c6ed02f37c5b2b20488046c8d90d75", + "reference": "08e27c97e4c6ed02f37c5b2b20488046c8d90d75", + "shasum": "" + }, + "require": { + "php": ">=5.2" + }, + "require-dev": { + "simpletest/simpletest": "dev-master#72de02a7b80c6bb8864ef9bf66d41d2f58f826bd" + }, + "type": "library", + "autoload": { + "psr-0": { + "HTMLPurifier": "library/" + }, + "files": [ + "library/HTMLPurifier.composer.php" + ], + "exclude-from-classmap": [ + "/library/HTMLPurifier/Language/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "LGPL-2.1-or-later" + ], + "authors": [ + { + "name": "Edward Z. Yang", + "email": "admin@htmlpurifier.org", + "homepage": "http://ezyang.com" + } + ], + "description": "Standards compliant HTML filter written in PHP", + "homepage": "http://htmlpurifier.org/", + "keywords": [ + "html" + ], + "support": { + "issues": "https://github.com/ezyang/htmlpurifier/issues", + "source": "https://github.com/ezyang/htmlpurifier/tree/master" + }, + "time": "2020-06-29T00:56:53+00:00" + }, { "name": "maennchen/zipstream-php", "version": "2.1.0", @@ -477,242 +531,6 @@ ], "time": "2017-10-23T01:57:42+00:00" }, - { - "name": "symfony/polyfill-iconv", - "version": "v1.20.0", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-iconv.git", - "reference": "c536646fdb4f29104dd26effc2fdcb9a5b085024" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-iconv/zipball/c536646fdb4f29104dd26effc2fdcb9a5b085024", - "reference": "c536646fdb4f29104dd26effc2fdcb9a5b085024", - "shasum": "" - }, - "require": { - "php": ">=7.1" - }, - "suggest": { - "ext-iconv": "For best performance" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-main": "1.20-dev" - }, - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Iconv\\": "" - }, - "files": [ - "bootstrap.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill for the Iconv extension", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "iconv", - "polyfill", - "portable", - "shim" - ], - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2020-10-23T14:02:19+00:00" - }, - { - "name": "symfony/polyfill-intl-grapheme", - "version": "v1.18.1", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-intl-grapheme.git", - "reference": "b740103edbdcc39602239ee8860f0f45a8eb9aa5" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-intl-grapheme/zipball/b740103edbdcc39602239ee8860f0f45a8eb9aa5", - "reference": "b740103edbdcc39602239ee8860f0f45a8eb9aa5", - "shasum": "" - }, - "require": { - "php": ">=5.3.3" - }, - "suggest": { - "ext-intl": "For best performance" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "1.18-dev" - }, - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Intl\\Grapheme\\": "" - }, - "files": [ - "bootstrap.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill for intl's grapheme_* functions", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "grapheme", - "intl", - "polyfill", - "portable", - "shim" - ], - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2020-07-14T12:35:20+00:00" - }, - { - "name": "symfony/polyfill-intl-normalizer", - "version": "v1.18.1", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-intl-normalizer.git", - "reference": "37078a8dd4a2a1e9ab0231af7c6cb671b2ed5a7e" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-intl-normalizer/zipball/37078a8dd4a2a1e9ab0231af7c6cb671b2ed5a7e", - "reference": "37078a8dd4a2a1e9ab0231af7c6cb671b2ed5a7e", - "shasum": "" - }, - "require": { - "php": ">=5.3.3" - }, - "suggest": { - "ext-intl": "For best performance" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "1.18-dev" - }, - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Intl\\Normalizer\\": "" - }, - "files": [ - "bootstrap.php" - ], - "classmap": [ - "Resources/stubs" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill for intl's Normalizer class and related functions", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "intl", - "normalizer", - "polyfill", - "portable", - "shim" - ], - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2020-07-14T12:35:20+00:00" - }, { "name": "symfony/polyfill-mbstring", "version": "v1.18.1", @@ -789,323 +607,6 @@ } ], "time": "2020-07-14T12:35:20+00:00" - }, - { - "name": "symfony/polyfill-php72", - "version": "v1.18.1", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-php72.git", - "reference": "639447d008615574653fb3bc60d1986d7172eaae" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-php72/zipball/639447d008615574653fb3bc60d1986d7172eaae", - "reference": "639447d008615574653fb3bc60d1986d7172eaae", - "shasum": "" - }, - "require": { - "php": ">=5.3.3" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "1.18-dev" - }, - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Php72\\": "" - }, - "files": [ - "bootstrap.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "polyfill", - "portable", - "shim" - ], - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2020-07-14T12:35:20+00:00" - }, - { - "name": "voku/anti-xss", - "version": "4.1.30", - "source": { - "type": "git", - "url": "https://github.com/voku/anti-xss.git", - "reference": "ff6e54f4a98ad1cd28f8b4a0f3c3f92f3c421f0a" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/voku/anti-xss/zipball/ff6e54f4a98ad1cd28f8b4a0f3c3f92f3c421f0a", - "reference": "ff6e54f4a98ad1cd28f8b4a0f3c3f92f3c421f0a", - "shasum": "" - }, - "require": { - "php": ">=7.0.0", - "voku/portable-utf8": "~5.4.50" - }, - "require-dev": { - "phpunit/phpunit": "~6.0 || ~7.0 || ~9.0" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "4.1.x-dev" - } - }, - "autoload": { - "psr-4": { - "voku\\helper\\": "src/voku/helper/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "EllisLab Dev Team", - "homepage": "http://ellislab.com/" - }, - { - "name": "Lars Moelleken", - "email": "lars@moelleken.org", - "homepage": "http://www.moelleken.org/" - } - ], - "description": "anti xss-library", - "homepage": "https://github.com/voku/anti-xss", - "keywords": [ - "anti-xss", - "clean", - "security", - "xss" - ], - "funding": [ - { - "url": "https://www.paypal.me/moelleken", - "type": "custom" - }, - { - "url": "https://github.com/voku", - "type": "github" - }, - { - "url": "https://opencollective.com/anti-xss", - "type": "open_collective" - }, - { - "url": "https://www.patreon.com/voku", - "type": "patreon" - }, - { - "url": "https://tidelift.com/funding/github/packagist/voku/anti-xss", - "type": "tidelift" - } - ], - "time": "2020-11-12T00:30:57+00:00" - }, - { - "name": "voku/portable-ascii", - "version": "1.5.6", - "source": { - "type": "git", - "url": "https://github.com/voku/portable-ascii.git", - "reference": "80953678b19901e5165c56752d087fc11526017c" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/voku/portable-ascii/zipball/80953678b19901e5165c56752d087fc11526017c", - "reference": "80953678b19901e5165c56752d087fc11526017c", - "shasum": "" - }, - "require": { - "php": ">=7.0.0" - }, - "require-dev": { - "phpunit/phpunit": "~6.0 || ~7.0 || ~9.0" - }, - "suggest": { - "ext-intl": "Use Intl for transliterator_transliterate() support" - }, - "type": "library", - "autoload": { - "psr-4": { - "voku\\": "src/voku/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Lars Moelleken", - "homepage": "http://www.moelleken.org/" - } - ], - "description": "Portable ASCII library - performance optimized (ascii) string functions for php.", - "homepage": "https://github.com/voku/portable-ascii", - "keywords": [ - "ascii", - "clean", - "php" - ], - "funding": [ - { - "url": "https://www.paypal.me/moelleken", - "type": "custom" - }, - { - "url": "https://github.com/voku", - "type": "github" - }, - { - "url": "https://opencollective.com/portable-ascii", - "type": "open_collective" - }, - { - "url": "https://www.patreon.com/voku", - "type": "patreon" - }, - { - "url": "https://tidelift.com/funding/github/packagist/voku/portable-ascii", - "type": "tidelift" - } - ], - "time": "2020-11-12T00:07:28+00:00" - }, - { - "name": "voku/portable-utf8", - "version": "5.4.50", - "source": { - "type": "git", - "url": "https://github.com/voku/portable-utf8.git", - "reference": "f14ed68ea9ced6639e71ca989c6d907892115ba0" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/voku/portable-utf8/zipball/f14ed68ea9ced6639e71ca989c6d907892115ba0", - "reference": "f14ed68ea9ced6639e71ca989c6d907892115ba0", - "shasum": "" - }, - "require": { - "php": ">=7.0.0", - "symfony/polyfill-iconv": "~1.0", - "symfony/polyfill-intl-grapheme": "~1.0", - "symfony/polyfill-intl-normalizer": "~1.0", - "symfony/polyfill-mbstring": "~1.0", - "symfony/polyfill-php72": "~1.0", - "voku/portable-ascii": "~1.5.6" - }, - "require-dev": { - "phpunit/phpunit": "~6.0 || ~7.0 || ~9.0" - }, - "suggest": { - "ext-ctype": "Use Ctype for e.g. hexadecimal digit detection", - "ext-fileinfo": "Use Fileinfo for better binary file detection", - "ext-iconv": "Use iconv for best performance", - "ext-intl": "Use Intl for best performance", - "ext-json": "Use JSON for string detection", - "ext-mbstring": "Use Mbstring for best performance" - }, - "type": "library", - "autoload": { - "psr-4": { - "voku\\": "src/voku/" - }, - "files": [ - "bootstrap.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "(Apache-2.0 or GPL-2.0)" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Hamid Sarfraz", - "homepage": "http://pageconfig.com/" - }, - { - "name": "Lars Moelleken", - "homepage": "http://www.moelleken.org/" - } - ], - "description": "Portable UTF-8 library - performance optimized (unicode) string functions for php.", - "homepage": "https://github.com/voku/portable-utf8", - "keywords": [ - "UTF", - "clean", - "php", - "unicode", - "utf-8", - "utf8" - ], - "funding": [ - { - "url": "https://www.paypal.me/moelleken", - "type": "custom" - }, - { - "url": "https://github.com/voku", - "type": "github" - }, - { - "url": "https://opencollective.com/portable-utf8", - "type": "open_collective" - }, - { - "url": "https://www.patreon.com/voku", - "type": "patreon" - }, - { - "url": "https://tidelift.com/funding/github/packagist/voku/portable-utf8", - "type": "tidelift" - } - ], - "time": "2020-11-12T00:17:47+00:00" } ], "packages-dev": [ @@ -4150,6 +3651,165 @@ ], "time": "2020-07-14T12:35:20+00:00" }, + { + "name": "symfony/polyfill-intl-grapheme", + "version": "v1.18.1", + "source": { + "type": "git", + "url": "https://github.com/symfony/polyfill-intl-grapheme.git", + "reference": "b740103edbdcc39602239ee8860f0f45a8eb9aa5" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/polyfill-intl-grapheme/zipball/b740103edbdcc39602239ee8860f0f45a8eb9aa5", + "reference": "b740103edbdcc39602239ee8860f0f45a8eb9aa5", + "shasum": "" + }, + "require": { + "php": ">=5.3.3" + }, + "suggest": { + "ext-intl": "For best performance" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.18-dev" + }, + "thanks": { + "name": "symfony/polyfill", + "url": "https://github.com/symfony/polyfill" + } + }, + "autoload": { + "psr-4": { + "Symfony\\Polyfill\\Intl\\Grapheme\\": "" + }, + "files": [ + "bootstrap.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony polyfill for intl's grapheme_* functions", + "homepage": "https://symfony.com", + "keywords": [ + "compatibility", + "grapheme", + "intl", + "polyfill", + "portable", + "shim" + ], + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2020-07-14T12:35:20+00:00" + }, + { + "name": "symfony/polyfill-intl-normalizer", + "version": "v1.18.1", + "source": { + "type": "git", + "url": "https://github.com/symfony/polyfill-intl-normalizer.git", + "reference": "37078a8dd4a2a1e9ab0231af7c6cb671b2ed5a7e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/polyfill-intl-normalizer/zipball/37078a8dd4a2a1e9ab0231af7c6cb671b2ed5a7e", + "reference": "37078a8dd4a2a1e9ab0231af7c6cb671b2ed5a7e", + "shasum": "" + }, + "require": { + "php": ">=5.3.3" + }, + "suggest": { + "ext-intl": "For best performance" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.18-dev" + }, + "thanks": { + "name": "symfony/polyfill", + "url": "https://github.com/symfony/polyfill" + } + }, + "autoload": { + "psr-4": { + "Symfony\\Polyfill\\Intl\\Normalizer\\": "" + }, + "files": [ + "bootstrap.php" + ], + "classmap": [ + "Resources/stubs" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony polyfill for intl's Normalizer class and related functions", + "homepage": "https://symfony.com", + "keywords": [ + "compatibility", + "intl", + "normalizer", + "polyfill", + "portable", + "shim" + ], + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2020-07-14T12:35:20+00:00" + }, { "name": "symfony/polyfill-php70", "version": "v1.18.1", @@ -4227,6 +3887,79 @@ ], "time": "2020-07-14T12:35:20+00:00" }, + { + "name": "symfony/polyfill-php72", + "version": "v1.18.1", + "source": { + "type": "git", + "url": "https://github.com/symfony/polyfill-php72.git", + "reference": "639447d008615574653fb3bc60d1986d7172eaae" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/polyfill-php72/zipball/639447d008615574653fb3bc60d1986d7172eaae", + "reference": "639447d008615574653fb3bc60d1986d7172eaae", + "shasum": "" + }, + "require": { + "php": ">=5.3.3" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.18-dev" + }, + "thanks": { + "name": "symfony/polyfill", + "url": "https://github.com/symfony/polyfill" + } + }, + "autoload": { + "psr-4": { + "Symfony\\Polyfill\\Php72\\": "" + }, + "files": [ + "bootstrap.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions", + "homepage": "https://symfony.com", + "keywords": [ + "compatibility", + "polyfill", + "portable", + "shim" + ], + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2020-07-14T12:35:20+00:00" + }, { "name": "symfony/polyfill-php73", "version": "v1.18.1", @@ -4836,7 +4569,7 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": "^7.2|^8.0", + "php": "^7.2||^8.0", "ext-ctype": "*", "ext-dom": "*", "ext-gd": "*", @@ -4852,5 +4585,5 @@ "ext-zlib": "*" }, "platform-dev": [], - "plugin-api-version": "1.1.0" + "plugin-api-version": "2.0.0" } diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index 31cc05af..e9de2ce6 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Writer; +use HTMLPurifier; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Cell\Cell; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; @@ -23,7 +24,6 @@ use PhpOffice\PhpSpreadsheet\Style\Style; use PhpOffice\PhpSpreadsheet\Worksheet\Drawing; use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; -use voku\helper\AntiXSS; class Html extends BaseWriter { @@ -1789,9 +1789,9 @@ class Html extends BaseWriter { $result = ''; if (!$this->isPdf && isset($pSheet->getComments()[$coordinate])) { - $sanitizer = new AntiXSS(); - $sanitizedString = $sanitizer->xss_clean($pSheet->getComment($coordinate)->getText()->getPlainText()); - if (!$sanitizer->isXssFound()) { + $sanitizer = new HTMLPurifier(); + $sanitizedString = $sanitizer->purify($pSheet->getComment($coordinate)->getText()->getPlainText()); + if ($sanitizedString !== '') { $result .= ''; $result .= '
' . nl2br($sanitizedString) . '
'; $result .= PHP_EOL; diff --git a/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php b/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php index 30d7af63..48aced02 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php @@ -10,15 +10,56 @@ use PhpOffice\PhpSpreadsheetTests\Functional; class XssVulnerabilityTest extends Functional\AbstractFunctional { + public function providerAcceptableMarkupRichText() + { + return [ + 'basic text' => ['Hello, I am safely viewing your site', 'Hello, I am safely viewing your site'], + 'link' => ["Google is here", 'Google is here'], + ]; + } + + /** + * @dataProvider providerAcceptableMarkupRichText + * + * @param string $safeTextString + * @param string $adjustedTextString + */ + public function testMarkupInComment($safeTextString, $adjustedTextString): void + { + $spreadsheet = new Spreadsheet(); + + $richText = new RichText(); + $richText->createText($safeTextString); + + $spreadsheet->getActiveSheet()->getCell('A1')->setValue('XSS Test'); + + $spreadsheet->getActiveSheet() + ->getComment('A1') + ->setText($richText); + + $filename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test'); + + $writer = IOFactory::createWriter($spreadsheet, 'Html'); + $writer->save($filename); + + $verify = file_get_contents($filename); + // Ensure that executable js has been stripped from the comments + self::assertStringContainsString($adjustedTextString, $verify); + } + public function providerXssRichText() { return [ - 'script tag' => [''], - 'javascript tag' => ['javascript:alert(1)'], - 'with unicode' => ['java\u0003script:alert(1)'], + 'script tag' => ["Hello, I am trying to your site"], + 'javascript tag' => ["CLICK"], + 'with unicode' => ['CLICK'], + 'inline css' => ['
  • '], + 'char value chevron' => ["\x3cscript src=http://www.example.com/malicious-code.js\x3e\x3c/script\x3e"], ]; } + private static $counter = 0; + /** * @dataProvider providerXssRichText * @@ -43,6 +84,8 @@ class XssVulnerabilityTest extends Functional\AbstractFunctional $writer->save($filename); $verify = file_get_contents($filename); + $counter = self::$counter++; + file_put_contents("verify{$counter}.html", $verify); // Ensure that executable js has been stripped from the comments self::assertStringNotContainsString($xssTextString, $verify); } From 1f2f2c79daca26f11de127403ff1b5b4ddb0893e Mon Sep 17 00:00:00 2001 From: Flinsch <220455+Flinsch@users.noreply.github.com> Date: Thu, 10 Dec 2020 21:49:53 +0100 Subject: [PATCH 14/30] 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 --- CHANGELOG.md | 3 ++- src/PhpSpreadsheet/Calculation/MathTrig.php | 8 ++++++++ tests/data/Calculation/MathTrig/ROUNDDOWN.php | 5 +++++ tests/data/Calculation/MathTrig/ROUNDUP.php | 5 +++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f07a84d1..11326638 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,7 +81,8 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed -- PrintArea causes exception [#1544](https://github.com/phpoffice/phpspreadsheet/pull/1544) +- PrintArea causes exception [#1544](https://github.com/phpoffice/phpspreadsheet/pull/1544) +- ROUNDUP and ROUNDDOWN return incorrect results for values of 0 [#1627](https://github.com/phpoffice/phpspreadsheet/pull/1627) - Calculation/DateTime Failure With PHP8 [#1661](https://github.com/phpoffice/phpspreadsheet/pull/1661) - Reader/Gnumeric Failure with PHP8 [#1662](https://github.com/phpoffice/phpspreadsheet/pull/1662) - ReverseSort bug, exposed but not caused by PHP8 [#1660](https://github.com/phpoffice/phpspreadsheet/pull/1660) diff --git a/src/PhpSpreadsheet/Calculation/MathTrig.php b/src/PhpSpreadsheet/Calculation/MathTrig.php index 7539659e..823f6ef2 100644 --- a/src/PhpSpreadsheet/Calculation/MathTrig.php +++ b/src/PhpSpreadsheet/Calculation/MathTrig.php @@ -1108,6 +1108,10 @@ class MathTrig $digits = Functions::flattenSingleValue($digits); if ((is_numeric($number)) && (is_numeric($digits))) { + if ($number == 0.0) { + return 0.0; + } + if ($number < 0.0) { return round($number - 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_DOWN); } @@ -1134,6 +1138,10 @@ class MathTrig $digits = Functions::flattenSingleValue($digits); if ((is_numeric($number)) && (is_numeric($digits))) { + if ($number == 0.0) { + return 0.0; + } + if ($number < 0.0) { return round($number + 0.5 * 0.1 ** $digits, $digits, PHP_ROUND_HALF_UP); } diff --git a/tests/data/Calculation/MathTrig/ROUNDDOWN.php b/tests/data/Calculation/MathTrig/ROUNDDOWN.php index b1b3ac71..1c2c67e1 100644 --- a/tests/data/Calculation/MathTrig/ROUNDDOWN.php +++ b/tests/data/Calculation/MathTrig/ROUNDDOWN.php @@ -1,6 +1,11 @@ Date: Thu, 10 Dec 2020 21:51:24 +0100 Subject: [PATCH 15/30] Update change log --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11326638..774590df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- ROUNDUP and ROUNDDOWN return incorrect results for values of 0 [#1627](https://github.com/phpoffice/phpspreadsheet/pull/1627) - Apply Column and Row Styles to Existing Cells [#1712](https://github.com/PHPOffice/PhpSpreadsheet/issues/1712) [PR #1721](https://github.com/PHPOffice/PhpSpreadsheet/pull/1721) - Resolve issues with defined names where worksheet doesn't exist (#1686)[https://github.com/PHPOffice/PhpSpreadsheet/issues/1686] and [#1723](https://github.com/PHPOffice/PhpSpreadsheet/issues/1723) - [PR #1742](https://github.com/PHPOffice/PhpSpreadsheet/pull/1742) - Fix for issue [#1735](https://github.com/PHPOffice/PhpSpreadsheet/issues/1735) Incorrect activeSheetIndex after RemoveSheetByIndex - [PR #1743](https://github.com/PHPOffice/PhpSpreadsheet/pull/1743) @@ -82,7 +83,6 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed - PrintArea causes exception [#1544](https://github.com/phpoffice/phpspreadsheet/pull/1544) -- ROUNDUP and ROUNDDOWN return incorrect results for values of 0 [#1627](https://github.com/phpoffice/phpspreadsheet/pull/1627) - Calculation/DateTime Failure With PHP8 [#1661](https://github.com/phpoffice/phpspreadsheet/pull/1661) - Reader/Gnumeric Failure with PHP8 [#1662](https://github.com/phpoffice/phpspreadsheet/pull/1662) - ReverseSort bug, exposed but not caused by PHP8 [#1660](https://github.com/phpoffice/phpspreadsheet/pull/1660) From e0feeca555a07c79e34a8a14d9149194f4053c01 Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 10 Dec 2020 13:02:36 -0800 Subject: [PATCH 16/30] 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. --- src/PhpSpreadsheet/Reader/Slk.php | 3 ++- tests/PhpSpreadsheetTests/Reader/SlkTest.php | 24 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Reader/Slk.php b/src/PhpSpreadsheet/Reader/Slk.php index 0e147376..e58ff2f6 100644 --- a/src/PhpSpreadsheet/Reader/Slk.php +++ b/src/PhpSpreadsheet/Reader/Slk.php @@ -9,6 +9,7 @@ use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException; use PhpOffice\PhpSpreadsheet\Shared\StringHelper; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Style\Border; +use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class Slk extends BaseReader { @@ -516,7 +517,7 @@ class Slk extends BaseReader $spreadsheet->createSheet(); } $spreadsheet->setActiveSheetIndex($this->sheetIndex); - $spreadsheet->getActiveSheet()->setTitle(basename($pFilename, '.slk')); + $spreadsheet->getActiveSheet()->setTitle(substr(basename($pFilename, '.slk'), 0, Worksheet::SHEET_TITLE_MAXIMUM_LENGTH)); // Loop through file $column = $row = ''; diff --git a/tests/PhpSpreadsheetTests/Reader/SlkTest.php b/tests/PhpSpreadsheetTests/Reader/SlkTest.php index 4c7cc513..e461557e 100644 --- a/tests/PhpSpreadsheetTests/Reader/SlkTest.php +++ b/tests/PhpSpreadsheetTests/Reader/SlkTest.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Reader; use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException; use PhpOffice\PhpSpreadsheet\Reader\Slk; +use PhpOffice\PhpSpreadsheet\Shared\File; use PhpOffice\PhpSpreadsheet\Style\Border; use PhpOffice\PhpSpreadsheet\Style\Fill; use PhpOffice\PhpSpreadsheet\Style\Font; @@ -12,6 +13,16 @@ class SlkTest extends \PHPUnit\Framework\TestCase { private static $testbook = __DIR__ . '/../../../samples/templates/SylkTest.slk'; + private $filename = ''; + + protected function teardown(): void + { + if ($this->filename) { + unlink($this->filename); + $this->filename = ''; + } + } + public function testInfo(): void { $reader = new Slk(); @@ -131,4 +142,17 @@ class SlkTest extends \PHPUnit\Framework\TestCase self::assertEquals('FFFF0000', $sheet->getCell('A1')->getStyle()->getFont()->getColor()->getARGB()); } + + public function testLongName(): void + { + $contents = file_get_contents(self::$testbook); + $this->filename = File::sysGetTempDir() + . '/123456789a123456789b123456789c12345.slk'; + file_put_contents($this->filename, $contents); + $reader = new Slk(); + $spreadsheet = $reader->load($this->filename); + $sheet = $spreadsheet->getActiveSheet(); + self::assertEquals('123456789a123456789b123456789c1', $sheet->getTitle()); + self::assertEquals('FFFF0000', $sheet->getCell('A1')->getStyle()->getFont()->getColor()->getARGB()); + } } From 1de03c2578bf6d816da2879b1bb3a8bf42134f2d Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 10 Dec 2020 22:04:00 +0100 Subject: [PATCH 17/30] Update change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 774590df..2eab866f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Resolve issue with SLK long filenames [#1612](https://github.com/PHPOffice/PhpSpreadsheet/issues/1612) - ROUNDUP and ROUNDDOWN return incorrect results for values of 0 [#1627](https://github.com/phpoffice/phpspreadsheet/pull/1627) - Apply Column and Row Styles to Existing Cells [#1712](https://github.com/PHPOffice/PhpSpreadsheet/issues/1712) [PR #1721](https://github.com/PHPOffice/PhpSpreadsheet/pull/1721) - Resolve issues with defined names where worksheet doesn't exist (#1686)[https://github.com/PHPOffice/PhpSpreadsheet/issues/1686] and [#1723](https://github.com/PHPOffice/PhpSpreadsheet/issues/1723) - [PR #1742](https://github.com/PHPOffice/PhpSpreadsheet/pull/1742) From 916b6888ebd5bf0ebbfdcc775b4089096d0bdc6a Mon Sep 17 00:00:00 2001 From: Jan Sverre Riksfjord Date: Thu, 10 Dec 2020 22:52:00 +0100 Subject: [PATCH 18/30] 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. --- src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php index b6a6fc39..8faa7ae2 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php @@ -1079,7 +1079,7 @@ class Worksheet extends WriterPart { $objWriter->writeAttribute('t', $mappedType); if (!$cellValue instanceof RichText) { - self::writeElementIf($objWriter, isset($pFlippedStringTable[$cellValue]), 'v', $pFlippedStringTable[$cellValue]); + self::writeElementIf($objWriter, isset($pFlippedStringTable[$cellValue]), 'v', $pFlippedStringTable[$cellValue] ?? ''); } else { $objWriter->writeElement('v', $pFlippedStringTable[$cellValue->getHashCode()]); } From 2c927b1ca5ae8991bfc21b8a8d852f07f060ef20 Mon Sep 17 00:00:00 2001 From: Max Kalyabin Date: Fri, 11 Dec 2020 01:10:01 +0300 Subject: [PATCH 19/30] fixes #1655 issue (#1656) Resolve problem with incorrectly defined hyperlinks --- src/PhpSpreadsheet/Reader/Xlsx/Hyperlinks.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Reader/Xlsx/Hyperlinks.php b/src/PhpSpreadsheet/Reader/Xlsx/Hyperlinks.php index 9e6aeaf7..106fd44e 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx/Hyperlinks.php +++ b/src/PhpSpreadsheet/Reader/Xlsx/Hyperlinks.php @@ -41,7 +41,7 @@ class Hyperlinks foreach (Coordinate::extractAllCellReferencesInRange($hyperlink['ref']) as $cellReference) { $cell = $worksheet->getCell($cellReference); if (isset($linkRel['id'])) { - $hyperlinkUrl = $this->hyperlinks[(string) $linkRel['id']]; + $hyperlinkUrl = $this->hyperlinks[(string) $linkRel['id']] ?? null; if (isset($hyperlink['location'])) { $hyperlinkUrl .= '#' . (string) $hyperlink['location']; } From 9cd39b8f4fe367e7f4b92bd9e94b87fed8352823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Despont?= Date: Thu, 10 Dec 2020 23:28:46 +0100 Subject: [PATCH 20/30] Add 'ps' suffix to printer settings resources IDs (#1690) * Add 'ps' suffix to printer settings resources IDs --- src/PhpSpreadsheet/Reader/Xlsx.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index 5dc48ff8..124cc3b2 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -1971,7 +1971,7 @@ class Xlsx extends BaseReader $unparsedPrinterSettings = &$unparsedLoadedData['sheets'][$docSheet->getCodeName()]['printerSettings']; foreach ($sheetPrinterSettings as $rId => $printerSettings) { - $rId = substr($rId, 3); // rIdXXX + $rId = substr($rId, 3) . 'ps'; // rIdXXX, add 'ps' suffix to avoid identical resource identifier collision with unparsed vmlDrawing $unparsedPrinterSettings[$rId] = []; $unparsedPrinterSettings[$rId]['filePath'] = self::dirAdd("$dir/$fileWorksheet", $printerSettings['Target']); $unparsedPrinterSettings[$rId]['relFilePath'] = (string) $printerSettings['Target']; From 2307df5387ee6fad6d2cc38109bd9d4ef5ce6204 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 10 Dec 2020 23:37:20 +0100 Subject: [PATCH 21/30] Update change log --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eab866f..df2a80ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Resolve Xlsx loader issue whe hyperlinks don't have a destination +- Resolve issues when printer settings resources IDs clash with drawing IDs - Resolve issue with SLK long filenames [#1612](https://github.com/PHPOffice/PhpSpreadsheet/issues/1612) - ROUNDUP and ROUNDDOWN return incorrect results for values of 0 [#1627](https://github.com/phpoffice/phpspreadsheet/pull/1627) - Apply Column and Row Styles to Existing Cells [#1712](https://github.com/PHPOffice/PhpSpreadsheet/issues/1712) [PR #1721](https://github.com/PHPOffice/PhpSpreadsheet/pull/1721) From 4133bcf1b4cf80ef3e3b501783c8d90484502abb Mon Sep 17 00:00:00 2001 From: Guilliam Xavier Date: Thu, 10 Dec 2020 23:46:56 +0100 Subject: [PATCH 22/30] Fix pixelsToPoints conversion (for HTML col width) (#1733) --- src/PhpSpreadsheet/Shared/Drawing.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/Drawing.php b/src/PhpSpreadsheet/Shared/Drawing.php index 25d6910d..4ff89595 100644 --- a/src/PhpSpreadsheet/Shared/Drawing.php +++ b/src/PhpSpreadsheet/Shared/Drawing.php @@ -98,7 +98,7 @@ class Drawing */ public static function pixelsToPoints($pValue) { - return $pValue * 0.67777777; + return $pValue * 0.75; } /** @@ -111,7 +111,7 @@ class Drawing public static function pointsToPixels($pValue) { if ($pValue != 0) { - return (int) ceil($pValue * 1.333333333); + return (int) ceil($pValue / 0.75); } return 0; From 8833c239fbfa2bbf49bd5768531dfbd59de72bc9 Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 10 Dec 2020 15:20:09 -0800 Subject: [PATCH 23/30] 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. --- src/PhpSpreadsheet/Style/Conditional.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Style/Conditional.php b/src/PhpSpreadsheet/Style/Conditional.php index 35ec479b..e4fe0acc 100644 --- a/src/PhpSpreadsheet/Style/Conditional.php +++ b/src/PhpSpreadsheet/Style/Conditional.php @@ -189,7 +189,7 @@ class Conditional implements IComparable /** * Set Conditions. * - * @param string[] $pValue Condition + * @param bool|float|int|string|string[] $pValue Condition * * @return $this */ From 3025824a483849e74eed2f0ef708c1596476f4ee Mon Sep 17 00:00:00 2001 From: oleibman Date: Thu, 17 Dec 2020 08:00:19 -0800 Subject: [PATCH 24/30] Merge pull request #1698 * Merge pull request #4 from PHPOffice/master * Restore Omitted Read XML Test --- src/PhpSpreadsheet/Reader/Xml.php | 8 ++- .../Reader/Xml/XmlOddTest.php | 70 +++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Xml/XmlOddTest.php diff --git a/src/PhpSpreadsheet/Reader/Xml.php b/src/PhpSpreadsheet/Reader/Xml.php index e4d251e2..11aa1df3 100644 --- a/src/PhpSpreadsheet/Reader/Xml.php +++ b/src/PhpSpreadsheet/Reader/Xml.php @@ -617,9 +617,11 @@ class Xml extends BaseReader ++$rowID; } - $xmlX = $worksheet->children($namespaces['x']); - if (isset($xmlX->WorksheetOptions)) { - (new PageSettings($xmlX, $namespaces))->loadPageSettings($spreadsheet); + if (isset($namespaces['x'])) { + $xmlX = $worksheet->children($namespaces['x']); + if (isset($xmlX->WorksheetOptions)) { + (new PageSettings($xmlX, $namespaces))->loadPageSettings($spreadsheet); + } } } ++$worksheetID; diff --git a/tests/PhpSpreadsheetTests/Reader/Xml/XmlOddTest.php b/tests/PhpSpreadsheetTests/Reader/Xml/XmlOddTest.php new file mode 100644 index 00000000..e0b43113 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xml/XmlOddTest.php @@ -0,0 +1,70 @@ +filename) { + unlink($this->filename); + $this->filename = ''; + } + } + + public function testWriteThenRead(): void + { + $xmldata = <<< 'EOT' + + + + Xml2003 Short Workbook + + + 2 + + + 9000 + 13860 + 240 + 75 + False + False + + + + + + + Test String 1 + + +
    +     +     +EOT; + $this->filename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test'); + file_put_contents($this->filename, $xmldata); + $reader = new Xml(); + $spreadsheet = $reader->load($this->filename); + self::assertEquals(1, $spreadsheet->getSheetCount()); + + $sheet = $spreadsheet->getActiveSheet(); + self::assertEquals('Sample Data', $sheet->getTitle()); + self::assertEquals('Test String 1', $sheet->getCell('A8')->getValue()); + + $props = $spreadsheet->getProperties(); + self::assertEquals('Xml2003 Short Workbook', $props->getTitle()); + self::assertEquals('2', $props->getCustomPropertyValue('myאInt')); + } +} From 51cb21297d53f1b2f9b0ed59fcd8a199159d4e74 Mon Sep 17 00:00:00 2001 From: Gianluca Giovinazzo Date: Thu, 17 Dec 2020 19:41:07 +0100 Subject: [PATCH 25/30] Fix for bug #1592 (UPDATED) (#1623) * Fix for Xls when BIFF8 SST (FCh) has bad Shared string length --- src/PhpSpreadsheet/Reader/Xls.php | 16 +++++++-- tests/PhpSpreadsheetTests/Reader/XlsTest.php | 35 +++++++++++++++++++ tests/data/Reader/XLS/bug1592.xls | Bin 0 -> 20992 bytes 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 tests/data/Reader/XLS/bug1592.xls diff --git a/src/PhpSpreadsheet/Reader/Xls.php b/src/PhpSpreadsheet/Reader/Xls.php index b5c92d8d..023806d6 100644 --- a/src/PhpSpreadsheet/Reader/Xls.php +++ b/src/PhpSpreadsheet/Reader/Xls.php @@ -2973,6 +2973,9 @@ class Xls extends BaseReader // offset within (spliced) record data $pos = 0; + // Limit global SST position, further control for bad SST Length in BIFF8 data + $limitposSST = 0; + // get spliced record data $splicedRecordData = $this->getSplicedRecordData(); @@ -2986,8 +2989,17 @@ class Xls extends BaseReader $nm = self::getInt4d($recordData, 4); $pos += 4; + // look up limit position + foreach ($spliceOffsets as $spliceOffset) { + // it can happen that the string is empty, therefore we need + // <= and not just < + if ($pos <= $spliceOffset) { + $limitposSST = $spliceOffset; + } + } + // loop through the Unicode strings (16-bit length) - for ($i = 0; $i < $nm; ++$i) { + for ($i = 0; $i < $nm && $pos < $limitposSST; ++$i) { // number of characters in the Unicode string $numChars = self::getUInt2d($recordData, $pos); $pos += 2; @@ -3020,7 +3032,7 @@ class Xls extends BaseReader // expected byte length of character array if not split $len = ($isCompressed) ? $numChars : $numChars * 2; - // look up limit position + // look up limit position - Check it again to be sure that no error occurs when parsing SST structure foreach ($spliceOffsets as $spliceOffset) { // it can happen that the string is empty, therefore we need // <= and not just < diff --git a/tests/PhpSpreadsheetTests/Reader/XlsTest.php b/tests/PhpSpreadsheetTests/Reader/XlsTest.php index da39f8b2..130374b3 100644 --- a/tests/PhpSpreadsheetTests/Reader/XlsTest.php +++ b/tests/PhpSpreadsheetTests/Reader/XlsTest.php @@ -43,4 +43,39 @@ class XlsTest extends AbstractFunctional self::assertEquals($sheet->getCell('A1')->getFormattedValue(), $newsheet->getCell('A1')->getFormattedValue()); self::assertEquals($sheet->getCell("$col$row")->getFormattedValue(), $newsheet->getCell("$col$row")->getFormattedValue()); } + + /** + * Test load Xls file with invalid length in SST map. + */ + public function testLoadXlsBug1592(): void + { + $filename = 'tests/data/Reader/XLS/bug1592.xls'; + $reader = new Xls(); + // When no fix applied, spreadsheet is not loaded + $spreadsheet = $reader->load($filename); + $sheet = $spreadsheet->getActiveSheet(); + $col = $sheet->getHighestColumn(); + $row = $sheet->getHighestRow(); + + $newspreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $newsheet = $newspreadsheet->getActiveSheet(); + $newcol = $newsheet->getHighestColumn(); + $newrow = $newsheet->getHighestRow(); + + self::assertEquals($spreadsheet->getSheetCount(), $newspreadsheet->getSheetCount()); + self::assertEquals($sheet->getTitle(), $newsheet->getTitle()); + self::assertEquals($sheet->getColumnDimensions(), $newsheet->getColumnDimensions()); + self::assertEquals($col, $newcol); + self::assertEquals($row, $newrow); + + $rowIterator = $sheet->getRowIterator(); + + foreach ($rowIterator as $row) { + foreach ($row->getCellIterator() as $cell) { + $valOld = $cell->getFormattedValue(); + $valNew = $newsheet->getCell($cell->getCoordinate())->getFormattedValue(); + self::assertEquals($valOld, $valNew); + } + } + } } diff --git a/tests/data/Reader/XLS/bug1592.xls b/tests/data/Reader/XLS/bug1592.xls new file mode 100644 index 0000000000000000000000000000000000000000..02413abcc2d884a2cb2d12810dbd849d0860f4fe GIT binary patch literal 20992 zcmeI)du$cieFyM!uiw}PY=bezkBcApfo+T*7%;{**ch)vX-Rw41qg3sx`^PG)ZbI9rt@2m@=r)^%UHbd|=AJ$C zz2|JTRZ_K$a<9hs&TnS!XXea#&D`Vr&flawf8+Nv|6WBr4yq{i$;$*4>x&2I{SH09 zQPJ|3FL_qK7SVeO{qphuCmt2jBebiz|rklr@X>k2({>w63K%A$Ze z9n6zuyH%$8ONzy-S2dX%N_i?yWt){MD)Dn@odF6Su-$TVsL!?xw5_;06{_RRb{*6jQutlRPQ~MF~jsQK`H6Pj7 zXwW@sC!Irs>g_*u^~OT`x7@D2s@AK(4dp${)to9 zWZpRr)Z-^ii}X^UZSi$PS0gZ=q`#bs0o@|qUSWd zpO+djq^fDFYFcALun^6iq>t^TNEN-Wq(U1~{j}ZUr`t8q`rcN&yn;$(r92ZARki%_ zO!g5lt*)r6G#9G^E%fPgBu-P9W;>QOPRMOB?a0adOPlv`FhO)9aX`v}>Ucs8qE~b*UcN zq?I;nRvq+grhSQ5fdSb!K59OiYR(WZ)jC>INSTAvdmT+)qqGqkrNm$XE%6?&gkMLg zuAePZYN<}CTWw0!wkygK@l<R^KAI%gS6&TJ^*^~@+f-xmq$}79oTA?$577i z@G`BeA4?}qFIPbi@{To*Hi{2YQ%#RJ^>S6-(9Uy=k0CO~M-&;0a>Sw?F)pQ%+j4P< zjPX82#^N0@KETNJIFTY_iH;cOh{*Ml9WhR;$o077M#eY=BV#F!SgIqI=7@1y7WsLb z9kFyrj54paZH6Pp879)mbi}qfV$|8Owxzy}70YqNawB3NOW9Ovv*b8$E>;Hytx>f<2>cf#nK$-CZ6+3B?o6L0U8dMyP-))JRwBL(HC zv=p?YAV@(vvUGKL{FSe1iT9H`%iexRffNN2*BIuQqAV#2q$rV!zOwlXTH^FJ64!nv z^`b$F28nAXOucB3qD9JiR#B%VPFo|zfD{8#3`jAS#I>L~Pcb6>R`PPDmZYivH1()i zqqX&UiUlbaq*zOe1&M3hboG|{(Z_M&9Km_e922#FJ}C~QIFRBjDGsDKk<#98|7|UC zUN921i#{nHq0|QxL^}00g0vB&jh3_#q>Uogy)*rGOgQgxZ^0Z>3LH}kNGTwtSW*f| zDI#SieDq99+y^mIDoEZf8eN`JK}xlxRFG0d`lR{s6D@Ji#z<)(rNJ?!fs|%RX&|ME zw6*rdoR++6e|?_3D}H^RHi5JWq)nE@SB<9C*d)@6xhLIP;$E0JCa=$;C9lt-Q_t(O z=+>S4EZ7oy{TAJi;C@TGdj2e7l68D+SF=c|}T@~uo%LFOYk}^Tc6zR>n-+QJd z?ui>|8%W+&sFt>Yw9S&XfwWDeuU>9=q9yL%8z~EC~MVkGGGu>L^YY`*mfRqDL4oEqclmk+ZNOwN?m+wb~ zb2(q(7%3N|T##}>%C)3ika9&DPQU&ZOYhM=kUyfP@KW%in)w-?e61g%{f)d9-0q0w zIb!*aSb-x}=!orb#EKlTVn?jR5i51Xb~<8Zj##-PR^f>4a>ObfF}l@lr77L6w_<#= zDl+YB9I;wQY>y*GH?6GC<1bnwjeU+7-Ql;^t9Qf>IARTs*g;2(Z-n{xg|C@I6f-3z z1SKW}B_;$VCS;YE5R{mZl$e&zm}k0da;`Sgc96D%v>l}Fmb4ut{t75vy;eNgr6n#a zM#=-p`?^cl)_EZ1SyCQIc_OWhd|0g|E`dhM2Pq$pDIcVKOUef+U!=j;*S2Vh%e9dT zKq>&K0Hgv-DgdcKq<=3S>CzkcvfmnDp8QTH;#RNF^YZfK&ofi6xbQR3g$;YyP*i#C5xoNT#6QrFW?X;wwAng>XCiP#xpe1gHj8q0v8AxRym0405NM#~* ze{bY>w8X8Ok;*|T2dNySa!V=)sa&MFr|8EQ+eOlrkj*+TC zss^bVq-sm52B})4(eLc)PYkCX_nVAV15yo0H6YbkQVmEoBHerSjYTbSZ_G%wAk~6Y z3sS8m)q+$j(w7I)zNjVc;~8lWNP9rq1JWK#+5^%ak$&>dsX$UV^|Jl7^wlI29O#+YOtgRkQzi9|I*CY zwZzvXMmh-6L6CT`qPh2e(2@>-Y6PiKq_2cd z{u4`lAK*>75Ad_x1(=cV{F~@_{nuuEZ=l%`JLHHRcEnm7u~tXyh$GhKh#hsrjyYn- z9kF&ttiusI;fS4d#5x_ZE=R1}5$kcpPB~(|j#!@~*6)Z7IAW(Au``a?Sx4-gBR1%W zop;18IATK)F}@GbL@`q~o1nxrL5XRC64PXrm?kJOO;Te1_@DnQxISE4bFMa0Gf2%K zHG|Y_NzEWNi}c<%cIRq|%ZiZ>fpiF@Lm(Zpq(dMb5{d3`{+gD!1RCiuNQXf>4ANms zIt!j% zt}l&r6r`gd9R=yAB^?Fns7U_N6kH1%=@>}IKspA}F-tlI(lL?zqbayKc+aV)$fYbp}2S^>3)B#e5NdD0j z+`1X*1V|@9IswuNOF9A436cDxDY(rv(n*j`f^-t3la_Q6q?01~M^kW1Y@|+*Izj3L zsne1=LFyFAKbnHucO!Ly)CE!(NL`lH1yYwt{?QcNYcNtbNZlZHgVb$F-5_;~Kcd$~qB1JW6g z&VY2rlFop1MkN1e3hp}_=`2WRK{^Z4SxY(#(pi!GqbayYZlrS{odf9{Narl+97yLx z@{gwAe!h_gK^g>U5Trp%8U$%jB>!j%zIrgyd63S7bRMMhmUJGZ^CJ02Q}8v3kuHFA z0i+8cU9hAJAYBm2KbnHCbc{3v(hx{PAPrg45J*EJ>CqHF&-(zww7-$}0Y)6Li;mc+ zBR1xUjXPo!j@YClHsy#-J7P1A*sLRV$q}1##O58b1xM_%BX-3RTXe*h9I<6b?5ZPn z%@Moqh^;tcHyp8>j@T_n?6xCz#}T{hh~0~b@qK_{ikT8K3?*h5O3W~nm|?5L3`2<- zmJ;J16~wvPNFyMPfHVTqh$W4HG$N9JR1lXHBV7dPB1jiOx@bukLAofC9u>r?$0g86 zqaclfGz!wFC5?hKDw2OR1($0hdH?sMug=Fn8Uty}lEy$96UjfCf@=#Sjl(gGgES7( zxFwB)#Q)!xuKc4ZxDGPX1V|GgO@K6ENfRJVh~ytl!8MzaCPA76X%eJKOPT~}QY8Oq z3a&4WGzHQWNK+t9S<)0pQzH3CQ*bS8q-l_*L7E0>+LES0nik1Fnu6!j%ZZnNE57InH^B~Pz(mY7~uc+zDKbnGDVk0epv;fiq zNDG#<0Mdd;{?QcNz8mQ>NS8sn4ANywx(w1~k^G}6xYuB$D`{G%zjmusXOAl(4z z21qw7=>|wQMDmZO;J&kwZh~|Zq?;h!w4|FL-4w|`nu2@eM!E&kEs$=3bjy-%fpkkG z|7Z&C=NsuZNVh?{4bp8(x((88k^G}6`0Bw(cR;!W(jAcQSkfJk?ug_cO~KbBM!E~q zU6AgAbk~yZf^=6T|7Z%n(lOFKknVwW52SmRbPuF^BI(f-KhOIBtF*t7_W|xZVhLE_(hF!yxt zgLL1L?t^q+B>xBzPB$Yx0Oy+-oO6uy2&6|KJp$>GB|QS^kx2e;);T{K=`l!; zL3#|*V@rAr(qobIH|y`wFM}*bDf%fG|45#L!Bq7JvFtbTANq9>emtd59;4?vnfpKY zr-yzqW`TafB}BhQ!@u*gNI%{5-+A!AEb5;-3w+KE_3y>;>I0-Ns#$eW_QSh?` Date: Thu, 17 Dec 2020 19:43:02 +0100 Subject: [PATCH 26/30] Update change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index df2a80ab..1f9c85fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Fix for Xls Reader when SST has a bad length [#1592](https://github.com/PHPOffice/PhpSpreadsheet/issues/1592) - Resolve Xlsx loader issue whe hyperlinks don't have a destination - Resolve issues when printer settings resources IDs clash with drawing IDs - Resolve issue with SLK long filenames [#1612](https://github.com/PHPOffice/PhpSpreadsheet/issues/1612) From 80078725248ec17fc528c064aa56970462eee0a6 Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Mon, 21 Dec 2020 17:16:19 +0100 Subject: [PATCH 27/30] Add nightly PHP 8.1 dev to github actions (#1763) --- .github/workflows/main.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f4e13a65..a01892ae 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,6 +10,7 @@ jobs: - '7.3' - '7.4' - '8.0' + - '8.1' name: PHP ${{ matrix.php-version }} @@ -37,7 +38,7 @@ jobs: - name: Delete composer lock file id: composer-lock - if: ${{ matrix.php-version == '8.0' }} + if: ${{ matrix.php-version == '8.0' || matrix.php-version == '8.1' }} run: | rm composer.lock echo "::set-output name=flags::--ignore-platform-reqs" From 607d3473e68ff6d4ccd9a6534b828f888921ed93 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Fri, 25 Dec 2020 17:22:31 +0100 Subject: [PATCH 28/30] Fix compatibility with ext-gd on php 8 (#1762) --- CHANGELOG.md | 1 + src/PhpSpreadsheet/Shared/Drawing.php | 4 +++- src/PhpSpreadsheet/Worksheet/MemoryDrawing.php | 8 +++++--- src/PhpSpreadsheet/Writer/Xls/Worksheet.php | 5 +++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f9c85fe..27fdd718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Fix for issue [#1735](https://github.com/PHPOffice/PhpSpreadsheet/issues/1735) Incorrect activeSheetIndex after RemoveSheetByIndex - [PR #1743](https://github.com/PHPOffice/PhpSpreadsheet/pull/1743) - Ensure that the list of shared formulae is maintained when an xlsx file is chunked with readFilter[Issue #169](https://github.com/PHPOffice/PhpSpreadsheet/issues/1669). - Fix for notice during accessing "cached magnification factor" offset [#1354](https://github.com/PHPOffice/PhpSpreadsheet/pull/1354) +- Fix compatibility with ext-gd on php 8 ### Security Fix (CVE-2020-7776) diff --git a/src/PhpSpreadsheet/Shared/Drawing.php b/src/PhpSpreadsheet/Shared/Drawing.php index 4ff89595..f41fb695 100644 --- a/src/PhpSpreadsheet/Shared/Drawing.php +++ b/src/PhpSpreadsheet/Shared/Drawing.php @@ -2,6 +2,8 @@ namespace PhpOffice\PhpSpreadsheet\Shared; +use GdImage; + class Drawing { /** @@ -152,7 +154,7 @@ class Drawing * * @param string $p_sFile Path to Windows DIB (BMP) image * - * @return resource + * @return GdImage|resource */ public static function imagecreatefrombmp($p_sFile) { diff --git a/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php b/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php index 22e09099..fb002114 100644 --- a/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php +++ b/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php @@ -2,6 +2,8 @@ namespace PhpOffice\PhpSpreadsheet\Worksheet; +use GdImage; + class MemoryDrawing extends BaseDrawing { // Rendering functions @@ -19,7 +21,7 @@ class MemoryDrawing extends BaseDrawing /** * Image resource. * - * @var resource + * @var GdImage|resource */ private $imageResource; @@ -62,7 +64,7 @@ class MemoryDrawing extends BaseDrawing /** * Get image resource. * - * @return resource + * @return GdImage|resource */ public function getImageResource() { @@ -72,7 +74,7 @@ class MemoryDrawing extends BaseDrawing /** * Set image resource. * - * @param resource $value + * @param GdImage|resource $value * * @return $this */ diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php index a1c258c0..a793128a 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Writer\Xls; +use GdImage; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Cell\DataType; use PhpOffice\PhpSpreadsheet\Cell\DataValidation; @@ -2254,7 +2255,7 @@ class Worksheet extends BIFFwriter */ public function insertBitmap($row, $col, $bitmap, $x = 0, $y = 0, $scale_x = 1, $scale_y = 1): void { - $bitmap_array = (is_resource($bitmap) ? $this->processBitmapGd($bitmap) : $this->processBitmap($bitmap)); + $bitmap_array = (is_resource($bitmap) || $bitmap instanceof GdImage ? $this->processBitmapGd($bitmap) : $this->processBitmap($bitmap)); [$width, $height, $size, $data] = $bitmap_array; // Scale the frame of the image. @@ -2460,7 +2461,7 @@ class Worksheet extends BIFFwriter /** * Convert a GD-image into the internal format. * - * @param resource $image The image to process + * @param GdImage|resource $image The image to process * * @return array Array with data and properties of the bitmap */ From e768cb0f191d5584acab01ad0cc932e8fc7d1ff1 Mon Sep 17 00:00:00 2001 From: oleibman Date: Fri, 25 Dec 2020 08:47:29 -0800 Subject: [PATCH 29/30] 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. --- docs/topics/reading-and-writing-to-file.md | 18 ++++ src/PhpSpreadsheet/Reader/Csv.php | 88 ++++++++++++++++-- tests/PhpSpreadsheetTests/Reader/CsvTest.php | 62 ++++++++++++ tests/data/Reader/CSV/escape.csv | 4 + tests/data/Reader/CSV/premiere.utf16be.csv | Bin 0 -> 112 bytes tests/data/Reader/CSV/premiere.utf16bebom.csv | Bin 0 -> 114 bytes tests/data/Reader/CSV/premiere.utf16le.csv | Bin 0 -> 112 bytes tests/data/Reader/CSV/premiere.utf16lebom.csv | Bin 0 -> 114 bytes tests/data/Reader/CSV/premiere.utf32be.csv | Bin 0 -> 224 bytes tests/data/Reader/CSV/premiere.utf32bebom.csv | Bin 0 -> 228 bytes tests/data/Reader/CSV/premiere.utf32le.csv | Bin 0 -> 224 bytes tests/data/Reader/CSV/premiere.utf32lebom.csv | Bin 0 -> 228 bytes tests/data/Reader/CSV/premiere.utf8.csv | 2 + tests/data/Reader/CSV/premiere.utf8bom.csv | 2 + tests/data/Reader/CSV/premiere.win1252.csv | 2 + 15 files changed, 170 insertions(+), 8 deletions(-) create mode 100644 tests/data/Reader/CSV/escape.csv create mode 100644 tests/data/Reader/CSV/premiere.utf16be.csv create mode 100644 tests/data/Reader/CSV/premiere.utf16bebom.csv create mode 100644 tests/data/Reader/CSV/premiere.utf16le.csv create mode 100644 tests/data/Reader/CSV/premiere.utf16lebom.csv create mode 100644 tests/data/Reader/CSV/premiere.utf32be.csv create mode 100644 tests/data/Reader/CSV/premiere.utf32bebom.csv create mode 100644 tests/data/Reader/CSV/premiere.utf32le.csv create mode 100644 tests/data/Reader/CSV/premiere.utf32lebom.csv create mode 100644 tests/data/Reader/CSV/premiere.utf8.csv create mode 100644 tests/data/Reader/CSV/premiere.utf8bom.csv create mode 100644 tests/data/Reader/CSV/premiere.win1252.csv diff --git a/docs/topics/reading-and-writing-to-file.md b/docs/topics/reading-and-writing-to-file.md index e55471a7..e1b7e3a2 100644 --- a/docs/topics/reading-and-writing-to-file.md +++ b/docs/topics/reading-and-writing-to-file.md @@ -458,6 +458,24 @@ $reader->setSheetIndex(0); $spreadsheet = $reader->load("sample.csv"); ``` +You may also let PhpSpreadsheet attempt to guess the input encoding. +It will do so based on a test for BOM (UTF-8, UTF-16BE, UTF-16LE, UTF-32BE, +or UTF-32LE), +or by doing heuristic tests for those encodings, falling back to a +specifiable encoding (default is CP1252) if all of those tests fail. + +```php +$reader = new \PhpOffice\PhpSpreadsheet\Reader\Csv(); +$encoding = \PhpOffice\PhpSpreadsheet\Reader\Csv::guessEncoding('sample.csv'); +// or, e.g. $encoding = \PhpOffice\PhpSpreadsheet\Reader\Csv::guessEncoding( +// 'sample.csv', 'ISO-8859-2'); +$reader->setInputEncoding($encoding); +$reader->setDelimiter(';'); +$reader->setEnclosure(''); +$reader->setSheetIndex(0); + +$spreadsheet = $reader->load('sample.csv'); +``` #### Read a specific worksheet diff --git a/src/PhpSpreadsheet/Reader/Csv.php b/src/PhpSpreadsheet/Reader/Csv.php index d6eb16b0..1495d102 100644 --- a/src/PhpSpreadsheet/Reader/Csv.php +++ b/src/PhpSpreadsheet/Reader/Csv.php @@ -9,6 +9,21 @@ use PhpOffice\PhpSpreadsheet\Spreadsheet; class Csv extends BaseReader { + const UTF8_BOM = "\xEF\xBB\xBF"; + const UTF8_BOM_LEN = 3; + const UTF16BE_BOM = "\xfe\xff"; + const UTF16BE_BOM_LEN = 2; + const UTF16BE_LF = "\x00\x0a"; + const UTF16LE_BOM = "\xff\xfe"; + const UTF16LE_BOM_LEN = 2; + const UTF16LE_LF = "\x0a\x00"; + const UTF32BE_BOM = "\x00\x00\xfe\xff"; + const UTF32BE_BOM_LEN = 4; + const UTF32BE_LF = "\x00\x00\x00\x0a"; + const UTF32LE_BOM = "\xff\xfe\x00\x00"; + const UTF32LE_BOM_LEN = 4; + const UTF32LE_LF = "\x0a\x00\x00\x00"; + /** * Input encoding. * @@ -90,12 +105,8 @@ class Csv extends BaseReader { rewind($this->fileHandle); - switch ($this->inputEncoding) { - case 'UTF-8': - fgets($this->fileHandle, 4) == "\xEF\xBB\xBF" ? - fseek($this->fileHandle, 3) : fseek($this->fileHandle, 0); - - break; + if (fgets($this->fileHandle, self::UTF8_BOM_LEN + 1) !== self::UTF8_BOM) { + rewind($this->fileHandle); } } @@ -213,7 +224,9 @@ class Csv extends BaseReader private function getNextLine() { $line = ''; - $enclosure = '(?escapeCharacter, '/') . ')' . preg_quote($this->enclosure, '/'); + $enclosure = ($this->escapeCharacter === '' ? '' + : ('(?escapeCharacter, '/') . ')')) + . preg_quote($this->enclosure, '/'); do { // Get the next line in the file @@ -307,7 +320,7 @@ class Csv extends BaseReader $this->fileHandle = fopen('php://memory', 'r+b'); $data = StringHelper::convertEncoding($entireFile, 'UTF-8', $this->inputEncoding); fwrite($this->fileHandle, $data); - rewind($this->fileHandle); + $this->skipBOM(); } } @@ -531,4 +544,63 @@ class Csv extends BaseReader return in_array($type, $supportedTypes, true); } + + private static function guessEncodingTestNoBom(string &$encoding, string &$contents, string $compare, string $setEncoding): void + { + if ($encoding === '') { + $pos = strpos($contents, $compare); + if ($pos !== false && $pos % strlen($compare) === 0) { + $encoding = $setEncoding; + } + } + } + + private static function guessEncodingNoBom(string $filename): string + { + $encoding = ''; + $contents = file_get_contents($filename); + self::guessEncodingTestNoBom($encoding, $contents, self::UTF32BE_LF, 'UTF-32BE'); + self::guessEncodingTestNoBom($encoding, $contents, self::UTF32LE_LF, 'UTF-32LE'); + self::guessEncodingTestNoBom($encoding, $contents, self::UTF16BE_LF, 'UTF-16BE'); + self::guessEncodingTestNoBom($encoding, $contents, self::UTF16LE_LF, 'UTF-16LE'); + if ($encoding === '' && preg_match('//u', $contents) === 1) { + $encoding = 'UTF-8'; + } + + return $encoding; + } + + private static function guessEncodingTestBom(string &$encoding, string $first4, string $compare, string $setEncoding): void + { + if ($encoding === '') { + if ($compare === substr($first4, 0, strlen($compare))) { + $encoding = $setEncoding; + } + } + } + + private static function guessEncodingBom(string $filename): string + { + $encoding = ''; + $first4 = file_get_contents($filename, false, null, 0, 4); + if ($first4 !== false) { + self::guessEncodingTestBom($encoding, $first4, self::UTF8_BOM, 'UTF-8'); + self::guessEncodingTestBom($encoding, $first4, self::UTF16BE_BOM, 'UTF-16BE'); + self::guessEncodingTestBom($encoding, $first4, self::UTF32BE_BOM, 'UTF-32BE'); + self::guessEncodingTestBom($encoding, $first4, self::UTF32LE_BOM, 'UTF-32LE'); + self::guessEncodingTestBom($encoding, $first4, self::UTF16LE_BOM, 'UTF-16LE'); + } + + return $encoding; + } + + public static function guessEncoding(string $filename, string $dflt = 'CP1252'): string + { + $encoding = self::guessEncodingBom($filename); + if ($encoding === '') { + $encoding = self::guessEncodingNoBom($filename); + } + + return ($encoding === '') ? $dflt : $encoding; + } } diff --git a/tests/PhpSpreadsheetTests/Reader/CsvTest.php b/tests/PhpSpreadsheetTests/Reader/CsvTest.php index 797f3f1d..e543ff48 100644 --- a/tests/PhpSpreadsheetTests/Reader/CsvTest.php +++ b/tests/PhpSpreadsheetTests/Reader/CsvTest.php @@ -275,4 +275,66 @@ EOF; $reader = new Csv(); $reader->load('tests/data/Reader/CSV/encoding.utf8.csvxxx'); } + + /** + * @dataProvider providerEscapes + */ + public function testInferSeparator(string $escape, string $delimiter): void + { + $reader = new Csv(); + $reader->setEscapeCharacter($escape); + $filename = 'tests/data/Reader/CSV/escape.csv'; + $reader->listWorksheetInfo($filename); + self::assertEquals($delimiter, $reader->getDelimiter()); + } + + public function providerEscapes() + { + return [ + ['\\', ';'], + ["\x0", ','], + [(version_compare(PHP_VERSION, '7.4') < 0) ? "\x0" : '', ','], + ]; + } + + /** + * @dataProvider providerGuessEncoding + */ + public function testGuessEncoding(string $filename): void + { + $reader = new Csv(); + $reader->setInputEncoding(Csv::guessEncoding($filename)); + $spreadsheet = $reader->load($filename); + $sheet = $spreadsheet->getActiveSheet(); + self::assertEquals('première', $sheet->getCell('A1')->getValue()); + self::assertEquals('sixième', $sheet->getCell('C2')->getValue()); + } + + public function providerGuessEncoding() + { + return [ + ['tests/data/Reader/CSV/premiere.utf8.csv'], + ['tests/data/Reader/CSV/premiere.utf8bom.csv'], + ['tests/data/Reader/CSV/premiere.utf16be.csv'], + ['tests/data/Reader/CSV/premiere.utf16bebom.csv'], + ['tests/data/Reader/CSV/premiere.utf16le.csv'], + ['tests/data/Reader/CSV/premiere.utf16lebom.csv'], + ['tests/data/Reader/CSV/premiere.utf32be.csv'], + ['tests/data/Reader/CSV/premiere.utf32bebom.csv'], + ['tests/data/Reader/CSV/premiere.utf32le.csv'], + ['tests/data/Reader/CSV/premiere.utf32lebom.csv'], + ['tests/data/Reader/CSV/premiere.win1252.csv'], + ]; + } + + public function testGuessEncodingDefltIso2(): void + { + $filename = 'tests/data/Reader/CSV/premiere.win1252.csv'; + $reader = new Csv(); + $reader->setInputEncoding(Csv::guessEncoding($filename, 'ISO-8859-2')); + $spreadsheet = $reader->load($filename); + $sheet = $spreadsheet->getActiveSheet(); + self::assertEquals('premičre', $sheet->getCell('A1')->getValue()); + self::assertEquals('sixičme', $sheet->getCell('C2')->getValue()); + } } diff --git a/tests/data/Reader/CSV/escape.csv b/tests/data/Reader/CSV/escape.csv new file mode 100644 index 00000000..a8b0c084 --- /dev/null +++ b/tests/data/Reader/CSV/escape.csv @@ -0,0 +1,4 @@ +a\"hello;hello;hello;\",b\"hello;hello;hello;\",c\"\hello;hello;hello;\" +a\"hello;hello;hello;\",b\"hello;hello;hello;\",c\"\hello;hello;hello;\",d +a\"hello;hello;hello;\",b\"hello;hello;hello;\",c\"\hello;hello;hello;\" +a\"hello;hello;hello;\",b\"hello;hello;hello;\",c\"\hello;hello;hello;\" diff --git a/tests/data/Reader/CSV/premiere.utf16be.csv b/tests/data/Reader/CSV/premiere.utf16be.csv new file mode 100644 index 0000000000000000000000000000000000000000..44c25684bc93576b6b0eac52da1170f3170f3ae6 GIT binary patch literal 112 zcmYL=u?>JQ3y4PCOUIC@0^;Nq~?_QnOFC>RlYSx7j*yt literal 0 HcmV?d00001 diff --git a/tests/data/Reader/CSV/premiere.utf16bebom.csv b/tests/data/Reader/CSV/premiere.utf16bebom.csv new file mode 100644 index 0000000000000000000000000000000000000000..2d63bbe12f6204cac31e9757141c625e75e47727 GIT binary patch literal 114 zcmYL=K@vbf3O-)j>%KXf$^V=$47Xuk` literal 0 HcmV?d00001 diff --git a/tests/data/Reader/CSV/premiere.utf16le.csv b/tests/data/Reader/CSV/premiere.utf16le.csv new file mode 100644 index 0000000000000000000000000000000000000000..a5bb1ff12e771e8628bf3c52930311bffbb3ca94 GIT binary patch literal 112 zcmYL=u?>JQ3`X59ggzQuxx_;(w<^xk27j*yt literal 0 HcmV?d00001 diff --git a/tests/data/Reader/CSV/premiere.utf32bebom.csv b/tests/data/Reader/CSV/premiere.utf32bebom.csv new file mode 100644 index 0000000000000000000000000000000000000000..83326b64e44768f5b574f8c70f3af8745838be05 GIT binary patch literal 228 zcmZwAxe0(!5Jl061v{}BOAs|Ma96Not5EM1ej)}3hS^@kbw|X61uc4XIFWh<^$O=T sMhs}+Y=gV@y>&PJ@Sk@%GL+a+A>Dm<{b`PUb7pw+x0kYxpZS~l027xPasU7T literal 0 HcmV?d00001 diff --git a/tests/data/Reader/CSV/premiere.utf32le.csv b/tests/data/Reader/CSV/premiere.utf32le.csv new file mode 100644 index 0000000000000000000000000000000000000000..64d29f13cfadcbf775041cafc2b5177f8dcad164 GIT binary patch literal 224 zcmZwAxe0(!5Jl061v{}BOAs|Ma96No%kbV{68XV_VYatL#DNhrCcMaALw&+|j};3B oIJ@AkeQ({(AO7>MMuiqPI^^Ad*PrItH)n@8rM=903iHhT0GW0dbpQYW literal 0 HcmV?d00001 diff --git a/tests/data/Reader/CSV/premiere.utf32lebom.csv b/tests/data/Reader/CSV/premiere.utf32lebom.csv new file mode 100644 index 0000000000000000000000000000000000000000..25617c6e9b2a51e78b00ac8e376509cb8c45f0e8 GIT binary patch literal 228 zcmZwAy9tC~5JlmMh3&*)uzyFCrH77%|{P_G;=C t&cB#3p@Xvx?%Ma(-TdJ{?>?x|U`LC*d+++w9Q)?{;Z12TvyQ?%Gap^<8FBys literal 0 HcmV?d00001 diff --git a/tests/data/Reader/CSV/premiere.utf8.csv b/tests/data/Reader/CSV/premiere.utf8.csv new file mode 100644 index 00000000..c6681201 --- /dev/null +++ b/tests/data/Reader/CSV/premiere.utf8.csv @@ -0,0 +1,2 @@ +première,second,troisième +Quatrième,cinquième,sixième diff --git a/tests/data/Reader/CSV/premiere.utf8bom.csv b/tests/data/Reader/CSV/premiere.utf8bom.csv new file mode 100644 index 00000000..4068e6c3 --- /dev/null +++ b/tests/data/Reader/CSV/premiere.utf8bom.csv @@ -0,0 +1,2 @@ +première,second,troisième +Quatrième,cinquième,sixième diff --git a/tests/data/Reader/CSV/premiere.win1252.csv b/tests/data/Reader/CSV/premiere.win1252.csv new file mode 100644 index 00000000..908cb88f --- /dev/null +++ b/tests/data/Reader/CSV/premiere.win1252.csv @@ -0,0 +1,2 @@ +premire,second,troisime +Quatrime,cinquime,sixime From a6240b0214456ef77a85d9c5469b6dc7d5836b55 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Fri, 25 Dec 2020 17:54:58 +0100 Subject: [PATCH 30/30] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27fdd718..23e7b13a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Added -- Nothing. +- CSV Reader - Best Guess for Encoding, and Handle Null-string Escape [#1647](https://github.com/PHPOffice/PhpSpreadsheet/issues/1647) ### Changed