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 crash loop for short message decode #71

Merged
merged 3 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/vumi2/transports/smpp/processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,11 @@ async def _handle_deliver_sm_esm_class(
if esm_class.type == EsmClassType.DEFAULT:
return False, None

content = pdu.params["short_message"].decode()
# The SMPP spec doesn't mention encodings at all for
# delivery reports, so assume they're plain ASCII and decode
# with latin1 to avoid decode errors.

content = pdu.params["short_message"].decode("latin1")
Buhle79 marked this conversation as resolved.
Show resolved Hide resolved
match = self.regex.match(content)
if not match:
logger.warning(
Expand All @@ -383,7 +387,7 @@ async def _handle_deliver_sm_body(
Try to decode the body as a delivery report, even if the esm_class doesn't
say it's a delivery report
"""
content = pdu.params["short_message"].decode()
content = pdu.params["short_message"].decode("latin1")
match = self.regex.match(content)
if not match:
return False, None
Expand Down
40 changes: 40 additions & 0 deletions tests/transports/smpp/test_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,3 +577,43 @@ async def test_short_message_multipart(sm_processer: ShortMessageProcessor):
assert msg.content == "part1part2"
assert msg.from_addr == "27820001001"
assert msg.to_addr == "123456"


async def test_invalid_delivery_report_esm_class_bad_encoding(
dr_processer: DeliveryReportProcesser, caplog
):
"""
If the ESM class says this is a delivery report, and it isn't UTF8-compatible,
then log a warning and don't return any event
"""
handled, event = await dr_processer.handle_deliver_sm(
DeliverSM(
esm_class=EsmClass(
EsmClassMode.DEFAULT, EsmClassType.SMSC_DELIVERY_RECEIPT
),
short_message=b"\xa4\xe8 bad bytes",
)
)
assert handled is False
assert event is None
[log] = [log for log in caplog.records if log.levelno >= logging.WARNING]

assert (
log.getMessage()
== "esm_class SMSC_DELIVERY_RECEIPT indicates delivery report, but regex"
" does not match content: ¤è bad bytes"
)


async def test_delivery_report_body_bad_encoding(dr_processer: DeliveryReportProcesser):
"""
If it is not valid UTF-8, don't throw a UnicodeDecodeError
"""
handled, event = await dr_processer.handle_deliver_sm(
DeliverSM(
esm_class=EsmClass(EsmClassMode.DEFAULT, EsmClassType.DEFAULT),
short_message=b"\xa4\xe8 bad bytes",
)
)
assert handled is False
assert event is None
Loading