podhmo / python-node-semver

python version of node-semver
MIT License
21 stars 15 forks source link

SemVer.__init__() can fail with UnboundLocalError on python-semver 0.4.2 #21

Closed tivek closed 5 years ago

tivek commented 5 years ago

I found this bug while testing Conan with a newer python-semver.

To reproduce:

[GCC 8.1.1 20180712 (Red Hat 8.1.1-5)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from semver import SemVer
>>> SemVer("1.a.1", loose=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/tivek/projects/python-semver/semver/__init__.py", line 340, in __init__
    other.append(k)
UnboundLocalError: local variable 'other' referenced before assignment

The last if-elif-else branch https://github.com/podhmo/python-semver/blob/c45457208647ea6f378f9f728b5ac55fd0a65dfc/semver/__init__.py#L334 leaves other unassigned.

Interestingly, other remains unused in the rest of the function. Is this var needed at all?

podhmo commented 5 years ago

hmm, I'm investigating it

podhmo commented 5 years ago

other is used for distributing extra tag. such as, +foo is build tag, and -foo is prerelease.

And, in node's semver 1.a.2 is invalid version.

> require("semver").SemVer("1.x.2", {loose: true})
TypeError: Invalid Version: 1.x.2
podhmo commented 5 years ago

0.5.0 is released.

climblinne commented 5 years ago

I think #24 touches this issue.

podhmo commented 5 years ago

hmm, skipping invalid input, is good or not good?....

climblinne commented 5 years ago

This would bring back the same functionality as before (0.2.0). I would also thought, that 1.a.4 could deliver an exception. But may be a user defined one. What is node-semver doing in this case?

podhmo commented 5 years ago

ah, yes, user's input is uncontrollable..

podhmo commented 5 years ago

:memo: node's semver is also rasising error

$ node
> var s = require("semver")
undefined
> s.maxSatisfying(["1.1.0"], "1.1.0", {loose: true})
'1.1.0'
> s.maxSatisfying(["1.1.0", "1.a.0"], "1.1.0", {loose: true})
TypeError: Invalid Version: 1.a.0
    at new SemVer (VENV/python-semver/node_modules/semver/semver.js:312:11)
    at Range.test (VENV/python-semver/node_modules/semver/semver.js:1137:15)
    at VENV/python-semver/node_modules/semver/semver.js:1202:18
    at Array.forEach (<anonymous>)
    at Function.maxSatisfying (VENV/python-semver/node_modules/semver/semver.js:1201:12)
podhmo commented 5 years ago

So, if ignoring invalid input, then handling in application (e.g. npm, conan, ...).

like a below

from semver import max_satisfying, make_semver

def partition(versions, loose=False):
    oks, ngs = [], []
    for v in versions:
        try:
            oks.append(make_semver(v, loose=loose))
        except ValueError as e:
            ngs.append((v, e))
    return oks, ngs

versions = ["1.a.1", "master", "X.2", "1.2.1", "1.3", "2.1"]
range_ = '1.3'
loose = True
oks, ngs = partition(versions, loose=loose)
print(oks)  # [<SemVer 2.0.0 >, <SemVer 1.2.1 >, <SemVer 1.3.0 >, <SemVer 2.1.0 >]
print(max_satisfying(oks, range_, loose=loose))  # 1.3.0
podhmo commented 5 years ago

hmm, another bug?

print(make_semver("X.2", loose=True))   # 2.0.0
podhmo commented 5 years ago

oh, sorry. commit message including "fix".

climblinne commented 5 years ago

Thanks, found my mistake. Closed pull request.