Add data type validation for WebHook payloads. - AP

Review Request #1353 — Created Aug. 18, 2018 and updated

Review Board
aaa, bbb


Early in the development in the WebHook support, we'd have certain
objects slipping in (like model instances) that broke encoding, and this
made it unsafe to support custom payloads in a non-controlled
environment (such as RBCommons). We attempted to serialize everything to
a set of safe data types, and have now done so for a while.

This change ensures this going forward by taking the payload being
dispatched and normalizing it first, converting everything to a simple
list of allowed primitives (and datetime objects, which are useful in
templates and are safe). The normalization process traverses the payload
in much the same way that the API serialization code does, but in a way
that's a bit more safe and specific to the needs of WebHooks. In the
process, this checks for any types that aren't expected and, if found,
logs the failure and aborts the dispatch.

While we're not going to trigger these exceptions on our in production
today, this does ensure that we don't accidentally introduce something
that will trigger it in the future, and that extension authors don't

Unit tests were updated to check the dispatch process and payload
content a bit more closely, ensuring that normalization and type
validation works correctly and testing more of the data sent in the
request. Much of this code was moved into a common helper to help
simplify tests. Other tests in the same file were also updated with
small fixes.

Testing Done:
All unit tests pass.

Manually tested events with a WebHook destination, checking the payloads
to make sure they contained what I expected and were received.

Reviewed at

None so far

Description From Last Updated

Dont delete

  2. reviewboard/notifications/ (Diff revision 1)

    Dont delete

Review request changed

Status: Re-opened