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.

Sign up for GitHub

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

Jump to bottom

Don't use assert for validation #48

Open
tucked opened this issue Oct 15, 2022 · 1 comment · May be fixed by #67
Open

Don't use assert for validation #48

tucked opened this issue Oct 15, 2022 · 1 comment · May be fixed by #67
Assignees
@ddmee

Comments

@tucked
Copy link

tucked commented Oct 15, 2022

polling2/polling2.py

Lines 166 to 171 in b9244a1

assert (timeout is not None or max_tries is not None) or poll_forever, \
('You did not specify a maximum number of tries or a timeout. Without either of these set, the polling '
'function will poll forever. If this is the behavior you want, pass "poll_forever=True"')
assert not ((timeout is not None or max_tries is not None) and poll_forever), \
'You cannot specify both the option to poll_forever and max_tries/timeout.' 

$ bandit polling2.py
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.8.10
[node_visitor]	WARNING	Unable to find qualified name for module: polling2.py
Run started:2022-10-14 22:09:04.481341

Test results:
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   Location: polling2.py:166:4
   More Info: https://bandit.readthedocs.io/en/1.7.4/plugins/b101_assert_used.html
165	
166	    assert (timeout is not None or max_tries is not None) or poll_forever, \
167	        ('You did not specify a maximum number of tries or a timeout. Without either of these set, the polling '
168	         'function will poll forever. If this is the behavior you want, pass "poll_forever=True"')
169	
170	    assert not ((timeout is not None or max_tries is not None) and poll_forever), \

--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   Location: polling2.py:170:4
   More Info: https://bandit.readthedocs.io/en/1.7.4/plugins/b101_assert_used.html
169	
170	    assert not ((timeout is not None or max_tries is not None) and poll_forever), \
171	        'You cannot specify both the option to poll_forever and max_tries/timeout.'
172	
173	    kwargs = kwargs or dict()

--------------------------------------------------

Code scanned:
	Total lines of code: 161
	Total lines skipped (#nosec): 0
	Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
	Total issues (by severity):
		Undefined: 0
		Low: 2
		Medium: 0
		High: 0
	Total issues (by confidence):
		Undefined: 0
		Low: 0
		Medium: 0
		High: 2
Files skipped (0):

Consider raising ValueError instead.
@ddmee ddmee self-assigned this Oct 15, 2022
@ddmee ddmee linked a pull request Oct 15, 2022 that will close this issue
Open
@ddmee ddmee linked a pull request Oct 15, 2022 that will close this issue
Bandit, release v1, use ifs over assert in src #67
Open
@ddmee
Copy link
Owner

ddmee commented Oct 15, 2022

hi @tucked thanks for raising this, I have raise a PR for this that should fix this issue. I'm going to still use assertionerror's in-case anyone is catching those. Though I think that's probably not likely, but you never know.

However, I'll use if statements instead of assert as that fixes bandit's complaint because if statemetns won't be optimised away in particularly runtime configurations.

Hope this satisfies you? If you want, have a look or comment on the PR. Cheers, Donal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ddmee ddmee

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bandit, release v1, use ifs over assert in src ddmee/polling2
2 participants
@ddmee @tucked