Skip to content

apigw clean up the invalid json parser #11968

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 6, 2024
Merged

apigw clean up the invalid json parser #11968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef1d0ea
start the cleanup
cloutierMat Nov 30, 2024
56cc71a
cover a few more cases
cloutierMat Dec 6, 2024
c8b0a0a
address pr comment
cloutierMat Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 26 additions & 30 deletions localstack-core/localstack/services/apigateway/next_gen/execute_api/integrations/mock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,48 +63,44 @@ def parse_invalid_json(self, body: str) -> dict:
CDK creates a MOCK OPTIONS route with in valid json. `{statusCode: 200}`
Aws probably has a custom token parser. We can implement one
at some point if we have user requests for it"""

def convert_null_value(value) -> str:
if (value := value.strip()) in ("null", ""):
return '""'
return value

try:
statuscode = ""
matched = re.match(r"^\s*{(.+)}\s*$", body).group(1)
splits = [m.strip() for m in matched.split(",")]
pairs = [m.strip() for m in matched.split(",")]
# TODO this is not right, but nested object would otherwise break the parsing
kvs = [s.split(":", maxsplit=1) for s in splits]
for kv in kvs:
assert len(kv) == 2
k, v = kv
k = k.strip()
v = v.strip()

assert k
assert v

if k == "statusCode":
statuscode = int(v)
key_values = [s.split(":", maxsplit=1) for s in pairs if s]
for key_value in key_values:
assert len(key_value) == 2
key, value = [convert_null_value(el) for el in key_value]

if key in ("statusCode", "'statusCode'", '"statusCode"'):
statuscode = int(value)
continue

if (first_char := k[0]) in "[{":
raise Exception
if first_char in "'\"":
assert len(k) > 2
assert k[-1] == first_char
k = k[1:-1]
assert (leading_key_char := key[0]) not in "[{"
if leading_key_char in "'\"":
assert len(key) >= 2
assert key[-1] == leading_key_char

if (v_first_char := v[0]) in "[{'\"":
assert len(v) > 2
if v_first_char == "{":
if (leading_value_char := value[0]) in "[{'\"":
assert len(value) >= 2
if leading_value_char == "{":
# TODO reparse objects
assert v[-1] == "}"
elif v_first_char == "[":
assert value[-1] == "}"
elif leading_value_char == "[":
# TODO validate arrays
assert v[-1] == "]"
assert value[-1] == "]"
else:
assert v[-1] == v_first_char
v = v[1:-1]

if k == "statusCode":
statuscode = int(v)
assert value[-1] == leading_value_char

return {"statusCode": statuscode}

except Exception as e:
LOG.debug(
"Error Parsing an invalid json, %s", e, exc_info=LOG.isEnabledFor(logging.DEBUG)
Expand Down
14 changes: 13 additions & 1 deletion tests/unit/services/apigateway/test_mock_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,25 @@ def test_custom_parser(self, create_default_context):
mock_integration = RestApiMockIntegration()

valid_templates = [
"{ statusCode: 200 }", # this is what the CDK creates when configuring CORS for rest apis
"{statusCode: 200,super{ f}oo: [ba r]}",
"{statusCode: 200, \"value\": 'goog'}",
"{statusCode: 200, foo}: [ba r]}",
"{statusCode: 200, foo'}: [ba r]}",
"{statusCode: 200, }foo: [ba r]}",
"{statusCode: 200, }foo: ''}",
'{statusCode: 200, " ": " "}',
'{statusCode: 200, "": ""}',
Comment on lines +66 to +67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also add a few cases with statusCode in single and double quotes, and maybe a few invalid with non closing quotes or something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thinking! that is indeed the most important objective 🤣

"{'statusCode': 200, '': ''}",
'{"statusCode": 200, "": ""}',
'{"statusCode": 200 , }',
'{"statusCode": 200 ,, }', # Because?? :cry-bear:
'{"statusCode": 200 , null: null }',
]
invalid_templates = [
"{\"statusCode': 200 }",
"{'statusCode\": 200 }",
"{'statusCode: 200 }",
"statusCode: 200",
"{statusCode: 200, {foo: [ba r]}",
# This test fails as we do not support nested objects
Expand All @@ -72,7 +84,7 @@ def test_custom_parser(self, create_default_context):
for valid_template in valid_templates:
ctx = create_default_context(body=valid_template)
response = mock_integration.invoke(ctx)
assert response["status_code"] == 200
assert response["status_code"] == 200, valid_template

for invalid_template in invalid_templates:
ctx = create_default_context(body=invalid_template)
Expand Down
Loading