Repository: falconry/falcon
Pull request #1688 (2020-02-27 15:16:25)

fix(SSEvent): Parameter validation in the constructor is too lenient
# Summary of Changes

This patch modifies the SSEvent initializer so that it explicitly checks whether a parameter is None. This avoids the problem of a parameter value having the wrong type, but being evaluated as falsey and thereby bypassing the type check.

# Related Issues

#1358 

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1688?src=pr&el=h1) Report
> Merging [#1688](https://codecov.io/gh/falconry/falcon/pull/1688?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/f94aad9aa659f62e2a3d2470653425e691099f50&el=desc) will **decrease** coverage by `10.46%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1688/graphs/tree.svg?width=650&height=150&src=pr&token=zOtkfNQ3h6)](https://codecov.io/gh/falconry/falcon/pull/1688?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##            master    #1688       +/-   ##
============================================
- Coverage   100.00%   89.53%   -10.47%     
============================================
  Files           47       43        -4     
  Lines         3527     2847      -680     
  Branches       548      425      -123     
============================================
- Hits          3527     2549      -978     
- Misses           0      280      +280     
- Partials         0       18       +18     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1688?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/asgi/response.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVzcG9uc2UucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/\_request\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvX3JlcXVlc3RfaGVscGVycy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/responders.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbmRlcnMucHk=) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [falcon/util/sync.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvc3luYy5weQ==) | `67.64% <0.00%> (-32.36%)` | :arrow_down: |
| [falcon/media/json.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL2pzb24ucHk=) | `68.96% <0.00%> (-31.04%)` | :arrow_down: |
| [falcon/media/validators/jsonschema.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL3ZhbGlkYXRvcnMvanNvbnNjaGVtYS5weQ==) | `69.04% <0.00%> (-30.96%)` | :arrow_down: |
| [falcon/testing/helpers.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvaGVscGVycy5weQ==) | `71.07% <0.00%> (-28.93%)` | :arrow_down: |
| [falcon/media/msgpack.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL21zZ3BhY2sucHk=) | `73.91% <0.00%> (-26.09%)` | :arrow_down: |
| [falcon/testing/resource.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvcmVzb3VyY2UucHk=) | `79.03% <0.00%> (-20.97%)` | :arrow_down: |
| [falcon/media/base.py](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL2Jhc2UucHk=) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| ... and [16 more](https://codecov.io/gh/falconry/falcon/pull/1688/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1688?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1688?src=pr&el=footer). Last update [f94aad9...d6dda3d](https://codecov.io/gh/falconry/falcon/pull/1688?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/falcon/asgi/structures.py b/falcon/asgi/structures.py
index 0cdaa1e..f68957d 100644
--- a/falcon/asgi/structures.py
+++ b/falcon/asgi/structures.py
@@ -84,22 +84,22 @@ class SSEvent:
         #   an error to be raised from the framework when it calls serialize()
         #   after the fact.
 
-        if data and not isinstance(data, bytes):
+        if data is not None and not isinstance(data, bytes):
             raise TypeError('data must be a byte string')
 
-        if text and not isinstance(text, str):
+        if text is not None and not isinstance(text, str):
             raise TypeError('text must be a string')
 
-        if event and not isinstance(event, str):
+        if event is not None and not isinstance(event, str):
             raise TypeError('event name must be a string')
 
-        if event_id and not isinstance(event_id, str):
+        if event_id is not None and not isinstance(event_id, str):
             raise TypeError('event_id must be a string')
 
-        if comment and not isinstance(comment, str):
+        if comment is not None and not isinstance(comment, str):
             raise TypeError('comment must be a string')
 
-        if retry and not isinstance(retry, int):
+        if retry is not None and not isinstance(retry, int):
             raise TypeError('retry must be an int')
 
         self.data = data


Test patch:
diff --git a/tests/asgi/test_sse.py b/tests/asgi/test_sse.py
index 116a970..ac55061 100644
--- a/tests/asgi/test_sse.py
+++ b/tests/asgi/test_sse.py
@@ -110,12 +110,18 @@ def test_invalid_event_values():
     with pytest.raises(TypeError):
         SSEvent(data=12345)
 
+    with pytest.raises(TypeError):
+        SSEvent(data=0)
+
     with pytest.raises(TypeError):
         SSEvent(text=b'notbytes')
 
     with pytest.raises(TypeError):
         SSEvent(text=23455)
 
+    with pytest.raises(TypeError):
+        SSEvent(text=0)
+
     with pytest.raises(TypeError):
         SSEvent(json=set()).serialize()
 
@@ -125,15 +131,24 @@ def test_invalid_event_values():
     with pytest.raises(TypeError):
         SSEvent(event=1234)
 
+    with pytest.raises(TypeError):
+        SSEvent(event=0)
+
     with pytest.raises(TypeError):
         SSEvent(event_id=b'idbytes')
 
     with pytest.raises(TypeError):
         SSEvent(event_id=52085)
 
+    with pytest.raises(TypeError):
+        SSEvent(event_id=0)
+
     with pytest.raises(TypeError):
         SSEvent(retry='5808.25')
 
+    with pytest.raises(TypeError):
+        SSEvent(retry='')
+
     with pytest.raises(TypeError):
         SSEvent(retry=5808.25)
 
@@ -143,6 +158,9 @@ def test_invalid_event_values():
     with pytest.raises(TypeError):
         SSEvent(comment=1234)
 
+    with pytest.raises(TypeError):
+        SSEvent(comment=0)
+
 
 def test_non_iterable():
     class SomeResource: