Skip to content

Commit

Permalink
Merge pull request #554 from crim-ca/fix-empty-input-string
Browse files Browse the repository at this point in the history
  • Loading branch information
fmigneault authored Sep 8, 2023
2 parents f7be768 + 97e800c commit ce06256
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 37 deletions.
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

0 comments on commit ce06256

Please sign in to comment.