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

Change: Assume UTC when no offset it specified #941

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Conversation

n-thumann
Copy link
Member

@n-thumann n-thumann commented Dec 8, 2023

What

This PR changes the behavior of handling datetime objects. Instead of assuming the local timezone, we should assume UTC instead.

Why

NVD always returns datetime objects in UTC, so we should mark them as those. The only documentation, I've found regarding this is datetime objects in the API response are always returned with a zero offset from UTC (see https://nvd.nist.gov/general/news/api-20-announcements).

Example:

➜  ~ curl -s "https://services.nvd.nist.gov/rest/json/cves/2.0?cveId=CVE-2022-45536" | jq ".vulnerabilities[0].cve.published"
"2022-11-22T21:15:11.103"

Current behavior:

➜  pontos git:(main) poetry run python            
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pontos.nvd.cve import CVEApi
>>> import asyncio
>>> from datetime import timezone
>>> async def foo(id):
...     async with CVEApi() as api:
...             return await api.cve(id)
... 
>>> asyncio.run(foo("CVE-2022-45536")).published
datetime.datetime(2022, 11, 22, 21, 15, 11, 103000)
>>> asyncio.run(foo("CVE-2022-45536")).published.timestamp()
1669148111.103

With this patch:

➜  pontos git:(assume_utc) poetry run python
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pontos.nvd.cve import CVEApi
>>> import asyncio
>>> from datetime import timezone
>>> async def foo(id):
...     async with CVEApi() as api:
...             return await api.cve(id)
... 
>>> asyncio.run(foo("CVE-2022-45536")).published
datetime.datetime(2022, 11, 22, 21, 15, 11, 103000, tzinfo=datetime.timezone.utc)
>>> asyncio.run(foo("CVE-2022-45536")).published.timestamp()
1669151711.103

References

The LoC form this PR have been first added in 2e397c3.

Checklist

  • Tests

Copy link

github-actions bot commented Dec 8, 2023

Conventional Commits Report

Type Number
Bug Fixes 1
Changed 1

🚀 Conventional commits found.

@n-thumann n-thumann marked this pull request as ready for review December 11, 2023 09:20
@n-thumann n-thumann requested a review from a team as a code owner December 11, 2023 09:20
@n-thumann n-thumann added the make release To trigger GitHub release action. label Dec 11, 2023
@bjoernricks bjoernricks enabled auto-merge (squash) December 11, 2023 09:21
@bjoernricks bjoernricks merged commit 08462b1 into main Dec 11, 2023
18 checks passed
@bjoernricks bjoernricks deleted the assume_utc branch December 11, 2023 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
make release To trigger GitHub release action.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants