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

Correct handling of Void response_bytes in OCSP #290

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

geitda
Copy link
Contributor

@geitda geitda commented Mar 25, 2025

response_bytes is optional, thus Void when omitted, which happens with some statuses (like 'unauthorized'). Make all properties return correct values in this case: None for everything except critical_extensions which is the empty set.
I wrote it with a guard that causes the indent to increase - could be rewritten as

if not self['response_bytes']
    self._processed_extensions = True
    return

if early return and repeating self._processed_extensions = True is deemed less bothersome than the increased indent.

The simple ternary operator fix for basic_ocsp_response and response_data may not be enough for every case as there are several more layers of unwrapping that need to be error-free, but looking at #246 and #262 this should fix the TypeError although user code will (still) have to check for None but the docstrings always indicated that possibility.

The test code I didn't write might include this - it's what a real 'unauthorized' response looks like (I got it from Let's Encrypt when querying an expired cert)

resp = ocsp.OCSPResponse.load(b'0\x03\n\x01\x06') # only 5 bytes

Yes, that short: a sequence with an enumerated '6' value, and the response_bytes is completely omitted.

'response_bytes' is optional, thus Void when omitted, which happens with some statuses (like 'unauthorized'). Make all properties return correct values in this case: 'None' for everything except 'critical_extensions' which is the empty set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant