From dd21ec77c73f9e05c6eb101482f03638d062ccb4 Mon Sep 17 00:00:00 2001 From: Kale Franz Date: Tue, 18 Sep 2018 08:56:27 -0500 Subject: [PATCH] resolve #7776 MalformedVersionError '~=' operator --- conda/core/index.py | 6 ++-- conda/exceptions.py | 22 +++++++++++--- conda/models/match_spec.py | 17 +++++------ conda/models/version.py | 52 ++++++++++++++++----------------- conda/resolve.py | 6 ++-- tests/models/test_match_spec.py | 13 +++++---- tests/models/test_version.py | 23 ++++++++++----- 7 files changed, 81 insertions(+), 58 deletions(-) diff --git a/conda/core/index.py b/conda/core/index.py index 30e5009976..b0aabba782 100644 --- a/conda/core/index.py +++ b/conda/core/index.py @@ -15,7 +15,7 @@ from ..base.context import context from ..common.compat import itervalues from ..common.io import ThreadLimitedThreadPoolExecutor, as_completed, dashlist, time_recorder -from ..exceptions import ChannelNotAllowed, InvalidVersionSpecError +from ..exceptions import ChannelNotAllowed, InvalidSpec from ..models.channel import Channel, all_channel_urls from ..models.match_spec import MatchSpec from ..models.records import EMPTY_LINK, PackageCacheRecord, PrefixRecord @@ -188,8 +188,8 @@ def push_spec(spec): def push_record(record): try: combined_depends = record.combined_depends - except InvalidVersionSpecError as e: - log.warning("Skipping %s due to InvalidVersionSpecError: %s", + except InvalidSpec as e: + log.warning("Skipping %s due to InvalidSpec: %s", record.record_id(), e._kwargs["invalid_spec"]) return push_spec(MatchSpec(record.name)) diff --git a/conda/exceptions.py b/conda/exceptions.py index 2047fe9d68..b9995cbf72 100644 --- a/conda/exceptions.py +++ b/conda/exceptions.py @@ -881,10 +881,24 @@ def __init__(self, path, placeholder, new_prefix, original_data_length, new_data super(BinaryPrefixReplacementError, self).__init__(message, **kwargs) -class InvalidVersionSpecError(CondaError): - def __init__(self, invalid_spec): - message = "Invalid version spec: %(invalid_spec)s" - super(InvalidVersionSpecError, self).__init__(message, invalid_spec=invalid_spec) +class InvalidSpec(CondaError, ValueError): + + def __init__(self, message, **kwargs): + super(InvalidSpec, self).__init__(message, **kwargs) + + +class InvalidVersionSpec(InvalidSpec): + def __init__(self, invalid_spec, details): + message = "Invalid version '%(invalid_spec)s': %(details)s" + super(InvalidVersionSpec, self).__init__(message, invalid_spec=invalid_spec, + details=details) + + +class InvalidMatchSpec(InvalidSpec): + def __init__(self, invalid_spec, details): + message = "Invalid spec '%(invalid_spec)s': %(details)s" + super(InvalidMatchSpec, self).__init__(message, invalid_spec=invalid_spec, + details=details) class EncodingError(CondaError): diff --git a/conda/models/match_spec.py b/conda/models/match_spec.py index 791bbfec58..5d31645218 100644 --- a/conda/models/match_spec.py +++ b/conda/models/match_spec.py @@ -22,7 +22,7 @@ from ..common.io import dashlist from ..common.path import expand from ..common.url import is_url, path_to_url, unquote -from ..exceptions import CondaValueError +from ..exceptions import CondaValueError, InvalidMatchSpec log = getLogger(__name__) @@ -372,11 +372,11 @@ def _hash_key(self): def __contains__(self, field): return field in self._match_components - @staticmethod - def _build_components(**kwargs): + def _build_components(self, **kwargs): not_fields = set(kwargs) - MatchSpec.FIELD_NAMES_SET if not_fields: - raise CondaValueError('Cannot match on field(s): %s' % not_fields) + raise InvalidMatchSpec(self._original_spec_str, + 'Cannot match on field(s): %s' % not_fields) _make_component = MatchSpec._make_component return frozendict(_make_component(key, value) for key, value in iteritems(kwargs)) @@ -506,7 +506,6 @@ def _parse_version_plus_build(v_plus_b): build = build and build.strip() else: version, build = v_plus_b, None - return version and version.replace(' ', ''), build @@ -600,7 +599,7 @@ def _parse_spec_str(spec_str): for match in m3b: key, _, value, _ = match.groups() if not key or not value: - raise CondaValueError("Invalid MatchSpec: %s" % spec_str) + raise InvalidMatchSpec(original_spec_str, "key-value mismatch in brackets") brackets[key] = value # Step 4. strip off parens portion @@ -645,9 +644,9 @@ def _parse_spec_str(spec_str): if m3: name, spec_str = m3.groups() if name is None: - raise CondaValueError("Invalid MatchSpec: %s" % spec_str) + raise InvalidMatchSpec(original_spec_str, "no package name found in '%s'" % spec_str) else: - raise CondaValueError("Invalid MatchSpec: %s" % spec_str) + raise InvalidMatchSpec(original_spec_str, "no package name found") # Step 7. otherwise sort out version + build spec_str = spec_str and spec_str.strip() @@ -657,7 +656,7 @@ def _parse_spec_str(spec_str): # name, version, build = _parse_legacy_dist(name) if spec_str: if '[' in spec_str: - raise CondaValueError("Invalid MatchSpec: %s" % spec_str) + raise InvalidMatchSpec(original_spec_str, "multiple brackets sections not allowed") version, build = _parse_version_plus_build(spec_str) diff --git a/conda/models/version.py b/conda/models/version.py index ad30c3bb39..d9b53b9e86 100644 --- a/conda/models/version.py +++ b/conda/models/version.py @@ -8,7 +8,7 @@ from .._vendor.toolz import excepts from ..common.compat import string_types, zip, zip_longest, text_type, with_metaclass -from ..exceptions import CondaValueError, InvalidVersionSpecError +from ..exceptions import InvalidVersionSpec log = getLogger(__name__) @@ -43,14 +43,6 @@ def __call__(cls, arg): return super(SingleStrArgCachingType, cls).__call__(arg) -class MalformedVersionError(CondaValueError): - - def __init__(self, version_string, details): - message = "Malformed version string '%(version_string)s': %(details)s" - super(MalformedVersionError, self).__init__(message, version_string=version_string, - details=details) - - @with_metaclass(SingleStrArgCachingType) class VersionOrder(object): """ @@ -168,7 +160,7 @@ def __init__(self, vstr): version = vstr.strip().rstrip().lower() # basic validity checks if version == '': - raise MalformedVersionError(vstr, "empty version string") + raise InvalidVersionSpec(vstr, "empty version string") invalid = not version_check_re.match(version) if invalid and '-' in version and '_' not in version: # Allow for dashes as long as there are no underscores @@ -176,7 +168,7 @@ def __init__(self, vstr): version = version.replace('-', '_') invalid = not version_check_re.match(version) if invalid: - raise MalformedVersionError(vstr, "invalid character(s)") + raise InvalidVersionSpec(vstr, "invalid character(s)") # when fillvalue == 0 => 1.1 == 1.1.0 # when fillvalue == -1 => 1.1 < 1.1.0 @@ -191,10 +183,10 @@ def __init__(self, vstr): elif len(version) == 2: # epoch given, must be an integer if not version[0].isdigit(): - raise MalformedVersionError(vstr, "epoch must be an integer") + raise InvalidVersionSpec(vstr, "epoch must be an integer") epoch = [version[0]] else: - raise MalformedVersionError(vstr, "duplicated epoch separator '!'") + raise InvalidVersionSpec(vstr, "duplicated epoch separator '!'") # find local version string version = version[-1].split('+') @@ -205,7 +197,7 @@ def __init__(self, vstr): # local version given self.local = version[1].replace('_', '.').split('.') else: - raise MalformedVersionError(vstr, "duplicated local version separator '+'") + raise InvalidVersionSpec(vstr, "duplicated local version separator '+'") # split version self.version = epoch + version[0].replace('_', '.').split('.') @@ -216,7 +208,7 @@ def __init__(self, vstr): for k in range(len(v)): c = version_split_re.findall(v[k]) if not c: - raise MalformedVersionError(vstr, "empty version component") + raise InvalidVersionSpec(vstr, "empty version component") for j in range(len(c)): if c[j].isdigit(): c[j] = int(c[j]) @@ -335,7 +327,7 @@ def apply_ops(cstop): # cstop: operators with lower precedence while stack and stack[-1] not in cstop: if len(output) < 2: - raise InvalidVersionSpecError(spec_str) + raise InvalidVersionSpec(spec_str, "cannot join single expression") c = stack.pop() r = output.pop() # Fuse expressions with the same operator; e.g., @@ -363,12 +355,12 @@ def apply_ops(cstop): elif item == ')': apply_ops('(') if not stack or stack[-1] != '(': - raise InvalidVersionSpecError(spec_str) + raise InvalidVersionSpec(spec_str, "expression must start with '('") stack.pop() else: output.append(item) if stack: - raise InvalidVersionSpecError(spec_str) + raise InvalidVersionSpec(spec_str, "unable to convert to expression tree: %s" % stack) return output[0] @@ -401,7 +393,7 @@ def untreeify(spec, _inand=False): # followed by a version string. It rejects expressions like # '<= 1.2' (space after operator), '<>1.2' (unknown operator), # and '<=!1.2' (nonsensical operator). -version_relation_re = re.compile(r'(=|==|!=|<=|>=|<|>)(?![=<>!])(\S+)$') +version_relation_re = re.compile(r'^(=|==|!=|<=|>=|<|>)(?![=<>!])(\S+)$') regex_split_re = re.compile(r'.*[()|,^$]') OPERATOR_MAP = { '==': op.__eq__, @@ -503,14 +495,15 @@ def get_matcher(self, vspec): vspec_str = text_type(vspec).strip() if vspec_str[0] == '^' or vspec_str[-1] == '$': if vspec_str[0] != '^' or vspec_str[-1] != '$': - raise InvalidVersionSpecError(vspec_str) + raise InvalidVersionSpec(vspec_str, "regex specs must start " + "with '^' and end with '$'") self.regex = re.compile(vspec_str) matcher = self.regex_match is_exact = False elif vspec_str[0] in OPERATOR_START: m = version_relation_re.match(vspec_str) if m is None: - raise InvalidVersionSpecError(vspec_str) + raise InvalidVersionSpec(vspec_str, "invalid operator") operator_str, vo_str = m.groups() if vo_str[-2:] == '.*': if operator_str in ("=", ">="): @@ -519,8 +512,11 @@ def get_matcher(self, vspec): vo_str = vo_str[:-2] operator_str = "!=startswith" else: - raise InvalidVersionSpecError(vspec_str) - self.operator_func = OPERATOR_MAP[operator_str] + raise InvalidVersionSpec(vspec_str, "invalid operator with '.*'") + try: + self.operator_func = OPERATOR_MAP[operator_str] + except KeyError: + raise InvalidVersionSpec(vspec_str, "invalid operator: %s" % operator_str) self.matcher_vo = VersionOrder(vo_str) matcher = self.operator_match is_exact = operator_str == "==" @@ -596,16 +592,20 @@ def get_matcher(self, vspec): elif vspec_str.startswith(('=', '<', '>', '!')): m = version_relation_re.match(vspec_str) if m is None: - raise InvalidVersionSpecError(vspec_str) + raise InvalidVersionSpec(vspec_str, "invalid operator") operator_str, vo_str = m.groups() - self.operator_func = OPERATOR_MAP[operator_str] + try: + self.operator_func = OPERATOR_MAP[operator_str] + except KeyError: + raise InvalidVersionSpec(vspec_str, "invalid operator: %s" % operator_str) self.matcher_vo = VersionOrder(vo_str) matcher = self.operator_match is_exact = operator_str == "==" elif vspec_str[0] == '^' or vspec_str[-1] == '$': if vspec_str[0] != '^' or vspec_str[-1] != '$': - raise InvalidVersionSpecError(vspec_str) + raise InvalidVersionSpec(vspec_str, "regex specs must start " + "with '^' and end with '$'") self.regex = re.compile(vspec_str) matcher = self.regex_match is_exact = False diff --git a/conda/resolve.py b/conda/resolve.py index 68d2e9488b..460202bda8 100644 --- a/conda/resolve.py +++ b/conda/resolve.py @@ -13,7 +13,7 @@ from .common.io import time_recorder from .common.logic import Clauses, minimal_unsatisfiable_subset from .common.toposort import toposort -from .exceptions import InvalidVersionSpecError, ResolvePackageNotFound, UnsatisfiableError +from .exceptions import InvalidSpec, ResolvePackageNotFound, UnsatisfiableError from .models.channel import Channel, MultiChannel from .models.enums import NoarchType from .models.match_spec import MatchSpec @@ -107,7 +107,7 @@ def v_fkey_(prec): filter[prec] = True try: depends = self.ms_depends(prec) - except InvalidVersionSpecError as e: + except InvalidSpec as e: val = filter[prec] = False else: val = filter[prec] = all(v_ms_(ms) for ms in depends) @@ -134,7 +134,7 @@ def is_valid_prec(prec): filter_out[prec] = False try: has_valid_deps = all(is_valid_spec(ms) for ms in self.ms_depends(prec)) - except InvalidVersionSpecError as e: + except InvalidSpec as e: val = filter_out[prec] = "invalid dep specs" else: val = filter_out[prec] = False if has_valid_deps else "invalid depends specs" diff --git a/tests/models/test_match_spec.py b/tests/models/test_match_spec.py index 138763e22a..be191f73bc 100644 --- a/tests/models/test_match_spec.py +++ b/tests/models/test_match_spec.py @@ -10,7 +10,7 @@ from conda.base.context import context from conda.cli.common import arg2spec, spec_from_line from conda.common.compat import on_win -from conda.exceptions import CondaValueError +from conda.exceptions import CondaValueError, InvalidMatchSpec, InvalidSpec from conda.models.channel import Channel from conda.models.dist import Dist from conda.models.records import PackageRecord @@ -291,19 +291,22 @@ def test_channel_matching(self): assert str(MatchSpec("defaults::*")) == "defaults::*" def test_matchspec_errors(self): - with pytest.raises(ValueError): + with pytest.raises(InvalidSpec): MatchSpec('blas [optional') - with pytest.raises(ValueError): + with pytest.raises(InvalidSpec): MatchSpec('blas [test=]') - with pytest.raises(ValueError): + with pytest.raises(InvalidSpec): MatchSpec('blas[invalid="1"]') if not on_win: # skipping on Windows for now. don't feel like dealing with the windows url path crud assert text_type(MatchSpec("/some/file/on/disk/package-1.2.3-2.tar.bz2")) == '*[url=file:///some/file/on/disk/package-1.2.3-2.tar.bz2]' + with pytest.raises(InvalidSpec): + MatchSpec("appdirs ~=1.4.3") + def test_dist(self): dst = Dist('defaults::foo-1.2.3-4.tar.bz2') a = MatchSpec(dst) @@ -696,7 +699,7 @@ def test_parse_hard(self): } def test_parse_errors(self): - with pytest.raises(CondaValueError): + with pytest.raises(InvalidMatchSpec): _parse_spec_str('!xyz 1.3') def test_parse_channel_subdir(self): diff --git a/tests/models/test_version.py b/tests/models/test_version.py index ac0a738c26..5d64633444 100644 --- a/tests/models/test_version.py +++ b/tests/models/test_version.py @@ -2,8 +2,9 @@ import unittest -from conda.exceptions import InvalidVersionSpecError +from conda.exceptions import InvalidVersionSpec from conda.models.version import VersionOrder, VersionSpec, normalized_version, ver_eval, treeify +import pytest class TestVersionSpec(unittest.TestCase): @@ -159,9 +160,9 @@ def test_ver_eval(self): self.assertEqual(ver_eval('1.2.3+4.5.6', '1.2.4+5*'), False) def test_ver_eval_errors(self): - self.assertRaises(InvalidVersionSpecError, ver_eval, '3.0.0', '><2.4.5') - self.assertRaises(InvalidVersionSpecError, ver_eval, '3.0.0', '!!2.4.5') - self.assertRaises(InvalidVersionSpecError, ver_eval, '3.0.0', '!') + self.assertRaises(InvalidVersionSpec, ver_eval, '3.0.0', '><2.4.5') + self.assertRaises(InvalidVersionSpec, ver_eval, '3.0.0', '!!2.4.5') + self.assertRaises(InvalidVersionSpec, ver_eval, '3.0.0', '!') def test_version_spec_1(self): v1 = VersionSpec('1.7.1') @@ -181,10 +182,10 @@ def test_version_spec_1(self): def test_version_spec_2(self): v1 = VersionSpec('( (1.5|((1.6|1.7), 1.8), 1.9 |2.0))|2.1') self.assertEqual(v1.spec, '1.5|1.6|1.7,1.8,1.9|2.0|2.1') - self.assertRaises(InvalidVersionSpecError, VersionSpec, '(1.5') - self.assertRaises(InvalidVersionSpecError, VersionSpec, '1.5)') - self.assertRaises(InvalidVersionSpecError, VersionSpec, '1.5||1.6') - self.assertRaises(InvalidVersionSpecError, VersionSpec, '^1.5') + self.assertRaises(InvalidVersionSpec, VersionSpec, '(1.5') + self.assertRaises(InvalidVersionSpec, VersionSpec, '1.5)') + self.assertRaises(InvalidVersionSpec, VersionSpec, '1.5||1.6') + self.assertRaises(InvalidVersionSpec, VersionSpec, '^1.5') def test_version_spec_3(self): v1 = VersionSpec('1.7.1*') @@ -270,3 +271,9 @@ def test_compound_versions(self): assert not vs.match('3.3.4') assert vs.match('3.4') assert vs.match('3.4a') + + def test_invalid_version_specs(self): + with pytest.raises(InvalidVersionSpec): + VersionSpec("~") + with pytest.raises(InvalidVersionSpec): + VersionSpec("^")