search

composer / semver

Semantic versioning utilities with the addition of version constraints parsing and checking.
MIT License
3.15k stars 76 forks source link

Previously invalid versions now valid (Bad aliases this time) #118

Closed ryanaslett closed 4 years ago

ryanaslett commented 4 years ago

I have found another subtle bug in the VersionParser.php.

The following constraint used to be invalid with the message the alias source must be an exact version, if it is a branch name you should prefix it with dev-

^1.0 as 0.9.1

I added that to the failingConstraints() data provider in VersionParserTest.php and ran the tests for 1.5.0, The test still passes (the constraint is rejected properly)

If you add that constraint to 1.7.1, the test fails, because it is not rejected.

Sorry that our flood of bad data is surfacing these subtle bugs. It only affects one project on packages.drupal.org, so we've just blitzed the metadata for those old versions as a workaround.

Opening the issue just in case it hits packagist / other repos harder.

Seldaek commented 4 years ago

I think that case was a voluntary change because really the constraint is fine as a constraint. It's only broken when parsed as an alias, but then those are only read on the root package, and composer will validate it correctly and error out so root package author can fix their stuff.

Breaking it when the package is used as a dependency isn't really needed or helpful. I hope this makes sense. /Cc @naderman any thoughts on this?

ryanaslett commented 4 years ago

I think the only time this actually affects somebody is if they had that as a dependency, and are also using an older version of composer that still thinks its not acceptable. Tthe user that reported it to us was on composer 1.10.6, so when it started re-appearing in the metadata, they started getting the error message.

Pretty edge caseish, and we only had one instance of it in our whole db, but just thought I'd mention it since it might show up again.

naderman commented 4 years ago

@Seldaek I'm not sure what your question is exactly. If the ^1.0 can be parsed correctly here, I'd say in a dependency this should be fine and not trigger an error as the alias only applies to the root package?

Seldaek commented 4 years ago

Yes that's exactly the point, so the change is ok, and yes it might indeed break older composer releases but we'll just have to live with that.