Usage of assert in http/client.py

[fbidu]

BPO	42060
Nosy	@benjaminp, @vedgar, @fbidu
PRs	bpo-42060: Remove assert from http/client.py #22737

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-10-17.12:53:09.125>
labels = ['library', '3.10']
title = 'Usage of assert in http/client.py'
updated_at = <Date 2020-10-20.02:34:38.077>
user = 'https://github.com/fbidu'

bugs.python.org fields:

activity = <Date 2020-10-20.02:34:38.077>
actor = 'benjamin.peterson'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-10-17.12:53:09.125>
creator = 'fbidu'
dependencies = []
files = []
hgrepos = []
issue_num = 42060
keywords = ['patch']
message_count = 3.0
messages = ['378810', '378903', '379078']
nosy_count = 3.0
nosy_names = ['benjamin.peterson', 'veky', 'fbidu']
pr_nums = ['22737']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue42060'
versions = ['Python 3.10']

[fbidu]

Hi all!

I was testing some static analysis tool and decided to use the HTTP module as testing ground. While running bandit at the client module, it detected 3 instances of using assert inside the code. Twice in the HTTPResponse class and once in the HTTPConnection class.

Now, I know that this will only cause any trouble when running python with the optimize settings turned on and if someone is that concerned about optimization, they probably won't be using the stdlib's HTTP implementation, but I think it would be fitting to fix this corner case.

I've written a PR that fixes this but I'm not sure if the raised exceptions and messages are ok

[vedgar]

If I understand correctly, those are _private methods_ (names start with underscore). In such cases, asserts can be completely fine, since the library writer controls the situations where the method is called. If it is never called with _UNKNOWN argument, assert is just that, assertion that it is so. (It would be different if it were a public method, since the library writer doesn't know it will never be called with _UNKNOWN.)

[benjaminp]

Are these assertion failures actually reachable due to malformed input? If so, there should be some tests to that effect. Otherwise, if they're actually "impossible" situations, I don't think this change is necessary.