Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SvnDriver: buildIdentifier must be of type int, string given #10646

Merged
merged 1 commit into from Mar 21, 2022

Conversation

glaubinix
Copy link
Contributor

@glaubinix glaubinix commented Mar 21, 2022

In SvnDriver.php line 403:
                                                                               
  [TypeError]                                                                  
  Composer\Repository\Vcs\SvnDriver::buildIdentifier(): Argument #2 ($revisio  
  n) must be of type int, string given, called in src/Composer/Reposi  
  tory/Vcs/SvnDriver.php on line 273  

@stof
Copy link
Contributor

@stof stof commented Mar 21, 2022

There is nothing in the regex ensuring that only integers are matched. So to me, a better fix would be to change the type in buildIdentifier instead (which does not actually care about getting a number anyway, as it only uses it to concatenate it with a string)

@glaubinix
Copy link
Contributor Author

@glaubinix glaubinix commented Mar 21, 2022

Right, but at the same time SVN revisions are always integers, so it makes sense for that method to only accept integers. Accepting string values would then allow the method to generate invalid build identifiers.

@glaubinix glaubinix force-pushed the f/svn-driver-build-identifier branch from b1c0cbc to 3491975 Compare Mar 21, 2022
@Seldaek
Copy link
Member

@Seldaek Seldaek commented Mar 21, 2022

Yeah I can see both ways are somewhat valid.. Given svn is somewhat on the EOL track, let's merge as is I guess :) Thanks

@Seldaek Seldaek merged commit bd89a67 into composer:main Mar 21, 2022
17 checks passed
@Seldaek Seldaek added this to the 2.3 milestone Mar 21, 2022
@Seldaek Seldaek added the Bug label Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants