Skip to content

Commit

Permalink
prevent defines without an = sign being accepted.
Browse files Browse the repository at this point in the history
  • Loading branch information
wxtim committed May 4, 2023
1 parent 89e8fd0 commit 5067143
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 4 deletions.
8 changes: 8 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ creating a new release entry be sure to copy & paste the span tag with the
updated. Only the first match gets replaced, so it's fine to leave the old
ones in. -->

## __cylc-rose-1.2.1 (<span actions:bind='release-date'>Upcoming</span>)__


### Fixes

[#225](https://github.com/cylc/cylc-rose/pull/225) - Prevent totally invalid
CLI --defines with no = sign.

## __cylc-rose-1.2.0 (<span actions:bind='release-date'>Released 2023-01-16</span>)__

### Fixes
Expand Down
8 changes: 8 additions & 0 deletions cylc/rose/entry_points.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@
dump_rose_log,
get_rose_vars_from_config_node,
identify_templating_section,
invalid_defines_check,
rose_config_exists,
rose_config_tree_loader,
merge_rose_cylc_suite_install_conf,
paths_to_pathlib,
get_cli_opts_node,
add_cylc_install_to_rose_conf_node_opts,
)
from cylc.flow.flags import cylc7_back_compat
from cylc.flow.hostuserutil import get_host


Expand Down Expand Up @@ -103,6 +105,12 @@ def get_rose_vars(srcdir=None, opts=None):
}
}
"""

# Check for definately invalid defines

if opts and hasattr(opts, 'defines') and not cylc7_back_compat:
invalid_defines_check(opts.defines)

# Set up blank page for returns.
config = {
'env': {},
Expand Down
44 changes: 40 additions & 4 deletions cylc/rose/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from cylc.flow.hostuserutil import get_host
from cylc.flow import LOG
from cylc.flow.exceptions import CylcError
import cylc.flow.flags as flags
from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros
from metomi.rose import __version__ as ROSE_VERSION
Expand All @@ -44,7 +45,11 @@
)


class MultipleTemplatingEnginesError(Exception):
class MultipleTemplatingEnginesError(CylcError):
...


class InvalidDefineError(CylcError):
...


Expand Down Expand Up @@ -325,11 +330,42 @@ def merge_rose_cylc_suite_install_conf(old, new):
return old


def invalid_defines_check(defines: List) -> None:
"""Check for defines which do not contain an = and therefore cannot be
valid
Examples:
# A single invalid define:
>>> import pytest
>>> with pytest.raises(InvalidDefineError, match=r'\\* foo'):
... invalid_defines_check(['foo'])
# Two invalid defines and one valid one:
>>> with pytest.raises(
... InvalidDefineError, match=r'\\* foo.*\\n.* \\* bar'
... ):
... invalid_defines_check(['foo', 'bar52', 'baz=442'])
# No invalid defines
>>> invalid_defines_check(['foo=12'])
"""
invalid_defines = []
for define in defines:
if parse_cli_defines(define) == 'Invalid':
invalid_defines.append(define)
if invalid_defines:
msg = 'Invalid Suite Defines (should contain an =)'
for define in invalid_defines:
msg += f'\n * {define}'
raise InvalidDefineError(msg)


def parse_cli_defines(define: str) -> Union[
bool, Tuple[
bool, str, Tuple[
List[Union[str, Any]],
Union[str, Any],
Union[str, Any]
Union[str, Any],
]
]:
"""Parse a define string.
Expand Down Expand Up @@ -381,7 +417,7 @@ def parse_cli_defines(define: str) -> Union[
# This seems like it ought to be an error,
# But behaviour is consistent with Rose 2019
# See:
return False
return "Invalid"

if match['state'] in ['!', '!!']:
LOG.warning(
Expand Down

0 comments on commit 5067143

Please sign in to comment.