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

Cannot seek stdin on pipe #496

Merged
merged 40 commits into from
Feb 18, 2022
Merged

Conversation

tylerwince
Copy link
Contributor

Fixes #495

@tylerwince
Copy link
Contributor Author

@ericwb This fixes the issue described in #495 regarding being able to pipe data into stdin for bandit using something like cat examples/assert.py | bandit -

Just implemented a Seeker class to wrap sys.stdin that implements a .seek method so we didn't have to add any additional conditionals to handle stdin vs file.

Happy to make changes if anyone has feedback or knows a better way to get around the OSError.

@tylerwince
Copy link
Contributor Author

@ericwb Just wanted to ping on this as it is fixing a bug and may be eligible for release in 1.6.1 in the next few days

@tylerwince
Copy link
Contributor Author

@ericwb just wanted to ping in this again

@Gee19
Copy link

Gee19 commented Aug 2, 2019

I'm working a project that uses bandit stdin and this PR works perfectly 👍

@tylerwince
Copy link
Contributor Author

@ericwb can this get merged and released? It's been sitting a while

@ericwb
Copy link
Member

ericwb commented Aug 6, 2019

@tylerwince Could you add a unit test recreating the issue described in #495 and the fix?

Copy link
Member

@ericwb ericwb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit test that recreates and exhibits the fix.

@ericwb
Copy link
Member

ericwb commented Mar 8, 2020

I'd like to merge once there is a unit test that verifies the fix.

nastra added a commit to nastra/flake8-bandit that referenced this pull request Feb 28, 2022
Fixes tylerwince#21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an `fdata` argument.
nastra added a commit to nastra/flake8-bandit that referenced this pull request Feb 28, 2022
Fixes tylerwince#21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an `fdata` argument and this just passes a `None` to make
things work with the latest version of bandit.
sathieu added a commit to sathieu/flake8-bandit that referenced this pull request Feb 28, 2022
Fixes: tylerwince#21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an `fdata` argument and this just passes a `None` to make
things work with the latest version of bandit.
nastra added a commit to nastra/flake8-bandit that referenced this pull request Mar 1, 2022
Fixes tylerwince#21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an `fdata` argument and this just passes a `None` to make
things work with the latest version of bandit.
nastra added a commit to nastra/flake8-bandit that referenced this pull request Mar 1, 2022
Fixes tylerwince#21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an `fdata` argument and this just passes a `None` to make
things work with the latest version of bandit.
@@ -15,7 +15,9 @@


class BanditNodeVisitor:
def __init__(self, fname, metaast, testset, debug, nosec_lines, metrics):
def __init__(
self, fname, fdata, metaast, testset, debug, nosec_lines, metrics
Copy link

@GriceTurrble GriceTurrble Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump for visibility, please see the number of issues that are being linked to this PR. This change to BanditNodeVisitor, which introduces new positional argument fdata in the middle of the signature, causes a backwards-incompatible break with flake8-bandit, which uses this object.

Ideally, this new argument should have been added to the end of the signature as an optional kwarg. Barring that, as small as this change is, this is now more than a patch update: I think this should have been released as 1.8.0 instead of 1.7.3.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bandit doesn't guarantee stability of its internals. Tools reaching in and trying to use them will break much like tools that reach directly into Flake8 for things they shouldn't

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.

cannot seek stdin on pipe
7 participants