[BUGFIX] Read project version from the DOM so "0.10" is not coerced to 0.1#1345
Open
CybotTM wants to merge 1 commit into
Open
[BUGFIX] Read project version from the DOM so "0.10" is not coerced to 0.1#1345CybotTM wants to merge 1 commit into
CybotTM wants to merge 1 commit into
Conversation
ffab307 to
fcc475c
Compare
fcc475c to
7d907c6
Compare
6b70577 to
6503239
Compare
6503239 to
c921c2c
Compare
Contributor
|
Thanks - I can't really judge the impact of this and hope @jaapio can give some feedback. I remember having stabbed at this and not being able to resolve this. |
c921c2c to
f346063
Compare
…o 0.1 A <project version="0.10"> in guides.xml was rendered as version 0.1 (title, objects.inv, every |version| substitution). XmlFileLoader parses guides.xml with XmlUtils::convertDomElementToArray(), which runs phpize() on every attribute value, coercing version-like strings into numbers: "0.10" becomes the float 0.1, "1.0" becomes 1, "13.0" becomes 13. Read the <project> attributes (all strings) straight from the DOM, and detach the element before the conversion so phpize never sees them. The version is now read correctly at the source instead of being coerced and patched up afterwards, so the beforeNormalization workaround in the Symfony config is removed. Writing the version directly (version="0.10") now just works. The previous "escaped version" workaround -- version="'3.0'" with single quotes to dodge phpize -- is no longer necessary, but existing guides.xml files may still use it, so the surrounding single quotes are still stripped (for version and release) to keep those files rendering 3.0 rather than the literal '3.0'. A regression fixture covers the quoted form. Reported on docs.typo3.org for netresearch/nr-vault and nr-llm (0.10 / 0.12). Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
f346063 to
c748708
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A
guides.xmlwith<project version="0.10">is rendered with the version0.1(title,objects.inv, every|version|substitution). Any round / trailing-zero two-part version is mangled:0.100.11.011.101.113.0130.9,13.4,1.2.3,mainIt surfaced on docs.typo3.org for
netresearch/nr-vaultandnetresearch/nr-llm, whose0.10/0.12releases were published under0.1.Cause
XmlFileLoaderparsesguides.xmlwithXmlUtils::convertDomElementToArray(), which runsphpize()on every attribute value.phpize("0.10")returns the float0.1(it isis_numeric), which then stringifies to"0.1".There was already a workaround — an escaped version syntax (
version="'3.0'", single-quoted to dodgephpize) plus abeforeNormalizationthat stripped the quotes again. But that runs afterphpizehas already discarded the digit (0.10→0.1), so it can never recover it.Fix
Read the
<project>attributes (all strings) directly from the DOM, and detach the element beforeconvertDomElementToArray()sophpizenever touches them. The version is read correctly at the source, instead of being coerced and patched up afterwards. The<guides>attributes that genuinely want coercion (links-are-relative,max-menu-depth, …) are unaffected.Because the root cause is fixed, the
beforeNormalizationworkaround in the Symfony config is removed — escaping is no longer needed to write a correct version.Backward compatibility
Projects that adopted the escaped version workaround for this very bug may still have
<project version="'3.0'">in theirguides.xml. To keep those rendering3.0(and not the literal'3.0'), the surrounding single quotes are still stripped forversionandrelease— the sametrim($value, "'")the removedbeforeNormalizationdid, now applied inXmlFileLoaderwhen the attributes are read. New files don't need it: write the version directly,version="0.10"/version="3.0".Before / after
<project version="0.10">as loaded byXmlFileLoader:Before
After
Tests
Two integration fixtures under
tests/Integration/tests/meta/:version-from-guides-xml— unescapedversion="0.10"(withrelease="3.0.0"as a non-coerced control); asserts the rendered|version|stays0.10. Passes with the fix and fails without it (without it:version 0.1).version-from-guides-xml-quoted— the legacy quoted formversion="'3.0'"/release="'3.0.0'"; asserts it still renders3.0/3.0.0, covering the backward-compatibility path above.Integration suite passes locally (228 tests). CI is green across PHP 8.1–8.3 (highest / locked / lowest): unit, integration, functional, Coding Standards, PHPStan and architecture checks.