Skip to content

Escape file name in checkstyle report output - #117

Open
ralucaStan wants to merge 1 commit into
sirbrillig:trunkfrom
ralucaStan:escape-checkstyle-filename
Open

Escape file name in checkstyle report output#117
ralucaStan wants to merge 1 commit into
sirbrillig:trunkfrom
ralucaStan:escape-checkstyle-filename

Conversation

@ralucaStan

Copy link
Copy Markdown

Summary

CheckstyleReporter escapes the message and source attributes but interpolates the file name into the <file name="..."> element raw. A path that contains an XML metacharacter (", <, >, &) therefore produces malformed checkstyle XML.

Impact

Any consumer that parses the checkstyle report — for example cs2pr, which turns it into GitHub PR annotations — fails to parse the output, so that run's findings silently don't render. File names come from the diff being linted, so this triggers whenever a changed file's path contains one of those characters. (The checkstyle reporter was added in #108.)

Fix

Escape the file name with the same escapeXml helper already used for the other attributes, and add a regression test (testXmlEscapingInFilename). ./vendor/bin/phpunit and composer lint pass.

Note

XmlReporter interpolates the file name (and its message/source) the same way; I've kept this PR scoped to the checkstyle format and am happy to follow up on the XML reporter separately if you'd like.


Prepared with AI assistance (Claude).

CheckstyleReporter escaped the message and source attributes but
interpolated the file name into the <file name="..."> element raw. A
path containing an XML metacharacter (", <, >, &) produced malformed
checkstyle XML, which downstream consumers such as cs2pr then fail to
parse. Escape the file name with the same escapeXml helper already used
for the other attributes, and cover it with a test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant