Repository: CTFd/CTFd
Pull request #2544 (2024-05-28 10:06:00)

Add none-check to time filter funcs
Add additional layer of None-checks to the time filter functions, to prevent 500s. Although we already covered the bug, it's possible we missed some edge cases, so I am fixing it closer to the source of the error

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/CTFd/CTFd/pull/2544?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) Report
Attention: Patch coverage is `25.00000%` with `6 lines` in your changes are missing coverage. Please review.
> Project coverage is 85.36%. Comparing base [(`162bca6`)](https://app.codecov.io/gh/CTFd/CTFd/commit/162bca6649d1cd17d6c4fb930010e53b183cd46d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) to head [(`6b6e51b`)](https://app.codecov.io/gh/CTFd/CTFd/commit/6b6e51bd770631abd6a800537f165b208558c6fb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd).
> Report is 11 commits behind head on master.

| [Files](https://app.codecov.io/gh/CTFd/CTFd/pull/2544?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) | Patch % | Lines |
|---|---|---|
| [CTFd/utils/dates/\_\_init\_\_.py](https://app.codecov.io/gh/CTFd/CTFd/pull/2544?src=pr&el=tree&filepath=CTFd%2Futils%2Fdates%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd#diff-Q1RGZC91dGlscy9kYXRlcy9fX2luaXRfXy5weQ==) | 25.00% | [6 Missing :warning: ](https://app.codecov.io/gh/CTFd/CTFd/pull/2544?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2544      +/-   ##
==========================================
- Coverage   87.57%   85.36%   -2.22%     
==========================================
  Files         155      153       -2     
  Lines        9650     9611      -39     
==========================================
- Hits         8451     8204     -247     
- Misses       1199     1407     +208     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/CTFd/CTFd/pull/2544?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd).

ColdHeat: I think overtime we should switch to using these Pascal Case aliases. I think I was wrong to use `import datetime` but I think the ambiguity of having `from datetime import datetime` is going to get confusing. `import datetime as dt` is going to lead to issues because we used the `dt` variable name. Thus I think using the cased aliases makes the most sense. The imports are broken up because on our isort version that's what it wants and I think it's easier to fix this later than to add the isort switch to fix this across the entire codebase. 

https://discuss.python.org/t/renaming-datetime-datetime-to-datetime-datetime/26279
https://github.com/PyCQA/isort/issues/1841

Patch:
diff --git a/CTFd/utils/dates/__init__.py b/CTFd/utils/dates/__init__.py
index 2f0b4b85..f26b4efe 100644
--- a/CTFd/utils/dates/__init__.py
+++ b/CTFd/utils/dates/__init__.py
@@ -1,5 +1,6 @@
-import datetime
 import time
+from datetime import datetime as DateTime
+from typing import Union
 
 from CTFd.utils import get_config
 
@@ -57,17 +58,30 @@ def view_after_ctf():
     return get_config("view_after_ctf")
 
 
-def unix_time(dt):
-    return int((dt - datetime.datetime(1970, 1, 1)).total_seconds())
+def unix_time(dt: DateTime) -> int:
+    if dt is None or not isinstance(dt, DateTime):
+        print("Invalid datetime object for time filter function.")
+        return None
+    return int((dt - DateTime(1970, 1, 1)).total_seconds())
 
 
-def unix_time_millis(dt):
-    return unix_time(dt) * 1000
+def unix_time_millis(dt: DateTime) -> int:
+    ut = unix_time(dt)
+    if ut is None:
+        return None
+    return ut * 1000
 
 
-def unix_time_to_utc(t):
-    return datetime.datetime.utcfromtimestamp(t)
+def unix_time_to_utc(t: Union[int, float]) -> DateTime:
+    if t is None:
+        print("Invalid datetime object for time filter function.")
+        return None
+    # TODO: The utcfromtimestamp() has been deprecated
+    return DateTime.utcfromtimestamp(t)
 
 
-def isoformat(dt):
+def isoformat(dt: DateTime) -> str:
+    if dt is None or not isinstance(dt, DateTime):
+        print("Invalid datetime object for time filter function.")
+        return None
     return dt.isoformat() + "Z"


Test patch:
diff --git a/tests/utils/test_ctftime.py b/tests/utils/test_ctftime.py
index f3027fa2..4755a5df 100644
--- a/tests/utils/test_ctftime.py
+++ b/tests/utils/test_ctftime.py
@@ -1,5 +1,17 @@
+from datetime import datetime as DateTime
+from datetime import timezone as TimeZone
+
+import pytest
+
 from CTFd.models import Solves
-from CTFd.utils.dates import ctf_ended, ctf_started
+from CTFd.utils.dates import (
+    ctf_ended,
+    ctf_started,
+    isoformat,
+    unix_time,
+    unix_time_millis,
+    unix_time_to_utc,
+)
 from CTFd.utils.modes import TEAMS_MODE
 from tests.helpers import (
     create_ctfd,
@@ -173,3 +185,55 @@ def test_ctf_ended():
             with ctftime.ended():
                 assert ctf_ended() is True
     destroy_ctfd(app)
+
+
+def test_unix_time():
+    """
+    Tests that the unix_time function returns the correct value and fails gracefully for strange inputs
+    """
+    assert unix_time(DateTime(2017, 1, 1)) == 1483228800
+    assert type(unix_time(DateTime(2017, 1, 1))) == int
+    assert unix_time(None) is None
+    assert unix_time("test") is None
+    assert unix_time(1) is None
+
+
+def test_unix_time_millis():
+    """
+    Tests that the unix_time function returns the correct value and fails gracefully for strange inputs
+    """
+    # Aware datetime object
+    assert unix_time_millis(DateTime(2017, 1, 1)) == 1483228800000
+    assert type(unix_time_millis(DateTime(2017, 1, 1))) == int
+    assert unix_time_millis(None) is None
+    assert unix_time_millis("test") is None
+    assert unix_time_millis(1) is None
+
+
+def test_unix_time_to_utc():
+    """
+    Tests that the unix_time function returns the correct value and fails gracefully for strange inputs
+    """
+    assert unix_time_to_utc(0) == DateTime(1970, 1, 1)
+    assert unix_time_to_utc(1483228800) == DateTime(2017, 1, 1)
+    assert type(unix_time_to_utc(1483228800)) == DateTime
+    assert unix_time_to_utc(None) is None
+    with pytest.raises(TypeError):
+        unix_time_to_utc("test")
+    with pytest.raises(TypeError):
+        unix_time_to_utc(DateTime(2017, 1, 1))
+
+
+def test_isoformat():
+    """
+    Tests that the unix_time function returns the correct value and fails gracefully for strange inputs
+    """
+    assert (
+        isoformat(DateTime(2017, 1, 1, tzinfo=TimeZone.utc))
+        == "2017-01-01T00:00:00+00:00Z"
+    )
+    assert isoformat(DateTime(2017, 1, 1)) == "2017-01-01T00:00:00Z"
+    assert isoformat(DateTime(2017, 1, 1, tzinfo=None)) == "2017-01-01T00:00:00Z"
+    assert isoformat(None) is None
+    assert isoformat("test") is None
+    assert isoformat(1) is None