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

fix schema validation prohibiting job execution with empty string input #554

Merged
merged 7 commits into from
Sep 8, 2023
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ Fixes:
------
- Fix broken `OpenAPI` schema link references to `OGC API - Processes` repository.
- Fix ``GET /providers/{provider_id}`` response using ``$schema`` instead of ``$id`` to provide its content schema.
- Fix `Job` creation failing when submitting an empty string as input for a `Process` that allows it due
to schema validation incorrectly preventing it.
- Fix human-readable `JSON`-like content cleanup to preserve sequences of quotes corresponding to valid empty strings.

.. _changes_4.30.1:

Expand Down
20 changes: 20 additions & 0 deletions tests/functional/test_wps_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
setup_config_with_celery,
setup_config_with_mongodb,
setup_config_with_pywps,
setup_mongodb_jobstore,
setup_mongodb_processstore
)
from weaver import xml_util
Expand All @@ -45,6 +46,7 @@ def setUp(self):
config = setup_config_with_pywps(config)
config = setup_config_with_celery(config)
self.process_store = setup_mongodb_processstore(config)
self.job_store = setup_mongodb_jobstore(config)
self.app = get_test_weaver_app(config=config, settings=settings)

# add processes by database Process type
Expand Down Expand Up @@ -146,6 +148,24 @@ def test_execute_allowed_demo(self):
resp.mustcontain("<wps:ProcessAccepted")
resp.mustcontain(f"PyWPS Process {HelloWPS.identifier}")

def test_execute_allowed_empty_string(self):
template = "service=wps&request=execute&version=1.0.0&identifier={}&datainputs=name="
params = template.format(HelloWPS.identifier)
url = self.make_url(params)
with contextlib.ExitStack() as stack_exec:
for mock_exec in mocked_execute_celery():
stack_exec.enter_context(mock_exec)
resp = self.app.get(url)
assert resp.status_code == 200 # FIXME: replace by 202 Accepted (?) https://github.com/crim-ca/weaver/issues/14
assert resp.content_type in ContentType.ANY_XML
resp.mustcontain("<wps:ExecuteResponse")
resp.mustcontain("<wps:ProcessAccepted")
resp.mustcontain(f"PyWPS Process {HelloWPS.identifier}")
loc = resp.xml.get("statusLocation")
job_id = loc.rsplit("/", 1)[-1].split(".", 1)[0]
job = self.job_store.fetch_by_id(job_id)
assert job.inputs[0]["data"] == ""

def test_execute_deployed_with_visibility_allowed(self):
headers = {"Accept": ContentType.APP_XML}
params_template = "service=wps&request=execute&version=1.0.0&identifier={}&datainputs=test_input=test"
Expand Down
33 changes: 19 additions & 14 deletions tests/test_owsexceptions.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import pytest

from weaver.owsexceptions import OWSException


def test_owsexceptions_json_formatter():
test_cases = [
@pytest.mark.parametrize(
["test", "expect"],
[
("\nLeading new-line",
"Leading new-line."),
("\nnew-lines\n\neverywhere\n",
Expand All @@ -19,6 +22,8 @@ def test_owsexceptions_json_formatter():
"Loads of dots remains..."),
("Contains some u''strings' not escaped",
"Contains some 'strings' not escaped."),
("Does not remove double quotes \"\" or '' that represent an empty string: {\"test\": \"\", \"other\": ''}.",
"Does not remove double quotes '' or '' that represent an empty string: {'test': '', 'other': ''}."),
("With \"double quotes\" not escaped.",
"With 'double quotes' not escaped."),
("With \'single quotes\' not escaped.",
Expand All @@ -37,17 +42,17 @@ def test_owsexceptions_json_formatter():
"Another long line, with many commas and newlines, but placed differently, just for the heck of it."),
("Literal new-lines are\\nresolved to space", "Literal new-lines are resolved to space."),
]

)
def test_owsexceptions_json_formatter(test, expect):
test_code = 418
test_status = f"{test_code} I'm a teapot"
for test, expect in test_cases:
json_body = OWSException.json_formatter(status=test_status, body=test, title="SomeCode", environ={})
assert json_body["code"] == "SomeCode"
assert json_body["error"]["code"] == test_code
assert json_body["error"]["status"] == test_status
result = json_body["description"]
assert json_body["description"] == expect, (
"Result does not match expected value"
f"\n Result: '{result}'"
f"\n Expect: '{expect}'"
)
json_body = OWSException.json_formatter(status=test_status, body=test, title="SomeCode", environ={})
assert json_body["code"] == "SomeCode"
assert json_body["error"]["code"] == test_code
assert json_body["error"]["status"] == test_status
result = json_body["description"]
assert json_body["description"] == expect, (
"Result does not match expected value"
f"\n Result: '{result}'"
f"\n Expect: '{expect}'"
)
15 changes: 15 additions & 0 deletions tests/wps_restapi/test_processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1951,6 +1951,21 @@ def test_execute_process_no_json_body(self):
assert resp.status_code == 400
assert resp.content_type == ContentType.APP_JSON

def test_execute_process_valid_empty_string(self):
"""
Ensure that a process expecting an input string parameter can be provided as empty (not resolved as "missing").
"""
path = f"/processes/{self.process_public.identifier}/jobs"
data = self.get_process_execute_template(test_input="")

with contextlib.ExitStack() as stack:
for exe in mocked_process_job_runner():
stack.enter_context(exe)
resp = self.app.post_json(path, params=data, headers=self.json_headers)
assert resp.status_code == 201, "Expected job submission without inputs created without error."
job = self.job_store.fetch_by_id(resp.json["jobID"])
assert job.inputs[0]["data"] == "", "Input value should be an empty string."

def test_execute_process_missing_required_params(self):
"""
Validate execution against missing parameters.
Expand Down
6 changes: 3 additions & 3 deletions weaver/processes/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -2861,10 +2861,10 @@ def wps2json_job_payload(wps_request, wps_process):
for input_value in input_list:
input_data = input_value.get("data")
input_href = input_value.get("href")
if input_data:
data["inputs"].append({"id": iid, "data": input_data})
elif input_href:
if input_href: # when href is provided, it must always be non-empty
data["inputs"].append({"id": iid, "href": input_href})
else: # no check if value to allow possible empty string, numeric zero or explicit null
data["inputs"].append({"id": iid, "data": input_data})
output_ids = list(wps_request.outputs)
for output in wps_process.outputs:
oid = output.identifier
Expand Down
38 changes: 31 additions & 7 deletions weaver/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3481,15 +3481,32 @@ def assert_sane_name(name, min_len=3, max_len=None):
raise InvalidIdentifierValue(f"Invalid name : {name}")


def clean_json_text_body(body, remove_newlines=True, remove_indents=True):
# type: (str, bool, bool) -> str
def clean_json_text_body(body, remove_newlines=True, remove_indents=True, convert_quotes=True):
# type: (str, bool, bool, bool) -> str
"""
Cleans a textual body field of superfluous characters to provide a better human-readable text in a JSON response.
"""
# cleanup various escape characters and u'' stings
replaces = [(",\n", ", "), ("\\n", " "), (" \n", " "), ("\n'", "'"), ("\"", "\'"),
("u\'", "\'"), ("u\"", "\'"), ("\'\'", "\'"), ("'. ", ""), ("'. '", ""),
("}'", "}"), ("'{", "{")]
replaces = [
(",\n", ", "),
("\\n", " "),
(" \n", " "),
("\n'", "'"),
("u\'", "\'"),
("u\"", "\""),
("'. ", ""),
("'. '", ""),
("}'", "}"),
("'{", "{"),
]
patterns = [
(re.compile(r"(\w+)('{2,})([\s\]\}\)]+)"), "\\1'\\3"),
(re.compile(r"([\s\[\{\(]+)('{2,})(\w+)"), "\\1'\\3"),
(re.compile(r"(\w+)(\"{2,})([\s\]\}\)]+)"), "\\1\"\\3"),
(re.compile(r"([\s\[\{\(]+)(\"{2,})(\w+)"), "\\1\"\\3"),
]
if convert_quotes:
replaces.extend([("\"", "\'")])
if remove_indents:
replaces.extend([("\\", " "), (" ", " ")])
else:
Expand All @@ -3501,6 +3518,8 @@ def clean_json_text_body(body, remove_newlines=True, remove_indents=True):
while any(rf in body for rf in replaces_from):
for _from, _to in replaces:
body = body.replace(_from, _to)
for _from, _to in patterns:
body = re.sub(_from, _to, body)

if remove_newlines:
body_parts = [p.strip() for p in body.split("\n") if p != ""] # remove new line and extra spaces
Expand All @@ -3511,8 +3530,13 @@ def clean_json_text_body(body, remove_newlines=True, remove_indents=True):
body_clean = body

# re-process without newlines to remove escapes created by concat of lines
if any(rf in body_clean for rf in replaces_from):
body_clean = clean_json_text_body(body_clean, remove_newlines=remove_newlines, remove_indents=remove_indents)
if any(rf in body_clean if isinstance(rf, str) else re.match(rf, body) for rf in replaces_from):
body_clean = clean_json_text_body(
body_clean,
remove_newlines=remove_newlines,
remove_indents=remove_indents,
convert_quotes=convert_quotes,
)
return body_clean


Expand Down
39 changes: 26 additions & 13 deletions weaver/wps_restapi/swagger_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1314,8 +1314,6 @@ class BoundingBoxInputType(ExtendedMappingSchema):
supportedCRS = SupportedCRSList()


# FIXME: support byte/binary type (string + format:byte) ?
# https://github.com/opengeospatial/ogcapi-processes/blob/master/openapi/schemas/processes-core/binaryInputValue.yaml
class AnyLiteralType(OneOfKeywordSchema):
"""
Submitted values that correspond to literal data.
Expand All @@ -1326,10 +1324,30 @@ class AnyLiteralType(OneOfKeywordSchema):
- :class:`AnyLiteralDefaultType`
"""
_one_of = [
ExtendedSchemaNode(Float(), description="Literal data type representing a floating point number."),
ExtendedSchemaNode(Integer(), description="Literal data type representing an integer number."),
ExtendedSchemaNode(Boolean(), description="Literal data type representing a boolean flag."),
ExtendedSchemaNode(String(), description="Literal data type representing a generic string."),
ExtendedSchemaNode(
Float(),
title="LiteralDataFloat",
description="Literal data type representing a floating point number.",
),
ExtendedSchemaNode(
Integer(),
title="LiteralDataInteger",
description="Literal data type representing an integer number.",
),
ExtendedSchemaNode(
Boolean(),
title="LiteralDataBoolean",
description="Literal data type representing a boolean flag.",
),
ExtendedSchemaNode(
# pylint: disable=C0301,line-too-long
# FIXME: support byte/binary type (string + format:byte) ?
# https://github.com/opengeospatial/ogcapi-processes/blob/master/openapi/schemas/processes-core/binaryInputValue.yaml
# see if we can use 'encoding' parameter available for below 'String' schema-type to handle this?
String(allow_empty=True), # valid to submit a process with empty string
title="LiteralDataString",
description="Literal data type representing a generic string.",
),
]


Expand Down Expand Up @@ -3490,14 +3508,9 @@ class ExecuteInputFile(AnyOfKeywordSchema):
# https://github.com/opengeospatial/ogcapi-processes/blob/master/openapi/schemas/processes-core/binaryInputValue.yaml
# FIXME: does not support bbox
# https://github.com/opengeospatial/ogcapi-processes/blob/master/openapi/schemas/processes-core/bbox.yaml
class ExecuteInputInlineValue(OneOfKeywordSchema):
class ExecuteInputInlineValue(AnyLiteralType):
title = "ExecuteInputInlineValue"
description = "Execute input value provided inline."
_one_of = [
ExtendedSchemaNode(Float(), title="ExecuteInputValueFloat"),
ExtendedSchemaNode(Integer(), title="ExecuteInputValueInteger"),
ExtendedSchemaNode(Boolean(), title="ExecuteInputValueBoolean"),
ExtendedSchemaNode(String(), title="ExecuteInputValueString"),
]


# https://github.com/opengeospatial/ogcapi-processes/blob/master/openapi/schemas/processes-core/inputValue.yaml
Expand Down
Loading