Skip to content

Commit 989a4ce

Browse files
committed
Html Writer Comments - Breaking Change
Fix PHPOffice#3954. In response to a Security Incident, a package was added to the project to sanitize the Html for comments attached to a cell. We have tried at least 2 different packages for this purpose, and users have raised legitimate concerns about both. I believe that adding a sanitizer package, although it addresses the problem, was overkill. Cell comments are a RichText object, and Html Writer already handled RichText cell *values*. Values did not figure in the Incident, and they surely would have done so if they were a problem because it is a lot easier to set up test cases for values than for comments. RichText values were not a problem because they were sanitized with `htmlspecialchars`; if comments were to use the same code that RichText was already using, it would likewise be safely sanitized. As an added bonus, the existing code for comments only uses the plaintext value, but the values code would allow the comments to be styled, just as they are for Xlsx. This is a breaking change - I don't think it will affect a lot of users, but there may be some. It is worth noting that the comment block before function `writeComment` has a link explaining what is being done. That link mentions only styling elements, not other possibilities like hyperlinks. At any rate, if people are putting styling tags (e.g. `<b>`) in their comments for this purpose, those will no longer work; the styling needs to be applied to the RichText elements. That will keep the user code the same regardless of the format of the intended output, which is certainly a good thing, but it is a break. If people are trying to insert non-styling tage (e.g. `<a>`), those will no longer work, just as there is no way to do it for Xlsx (although Excel itself may convert the raw text to a hyperlink, but that's out of scope, at least for now). I also note that, even with the current code, I haven't yet found a way to keep the Html comment in place long enough to actually click on any hyperlinks. The main package being dropped brings several other packages along with it, so the project as a whole will have a slightly lighter footprint than before. The existing `XssVulnerability` test cases are all preserved. Although the final result of the sanitizing changes, it can easily be seen that the results are all harmless.
1 parent 9a94aea commit 989a4ce

File tree

6 files changed

+311
-690
lines changed

6 files changed

+311
-690
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org).
77

88
## Unreleased - TBD
99

10+
### BREAKING CHANGE
11+
12+
- Writing of cell comments to Html will now sanitize all Html tags within the comment, so the tags will be rendered as plaintext and have no other effects when rendered. Styling can be achieved by using the Font property of of the TextRuns which make up the comment, as is already the cases for Xlsx. [PR #3957](https://github.com/PHPOffice/PhpSpreadsheet/pull/3957)
13+
1014
### Added
1115

1216
- Default Style Alignment Property (workaround for bug in non-Excel spreadsheet apps) [Issue #3918](https://github.com/PHPOffice/PhpSpreadsheet/issues/3918) [PR #3924](https://github.com/PHPOffice/PhpSpreadsheet/pull/3924)

composer.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,7 @@
8484
"markbaker/matrix": "^3.0",
8585
"psr/http-client": "^1.0",
8686
"psr/http-factory": "^1.0",
87-
"psr/simple-cache": "^1.0 || ^2.0 || ^3.0",
88-
"voku/anti-xss": "^4.1"
87+
"psr/simple-cache": "^1.0 || ^2.0 || ^3.0"
8988
},
9089
"require-dev": {
9190
"dealerdirect/phpcodesniffer-composer-installer": "dev-main",

0 commit comments

Comments
 (0)