-
Notifications
You must be signed in to change notification settings - Fork 250
Remove extraneous reassignments in output #166
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
Remove extraneous reassignments in output #166
Conversation
The output should consist of the path from the source to the sink. Anything which happens after the source reaches the sink is irrelevant and just makes the output longer and confusing to interpret. None of the lines removed from the tests actually affected the vulnerability chain. Perhaps this should be dealt with somewhere in the definition_chain or vulnerability functions: here we just trim the chain upon reaching the sink in the vulnerability_helper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
This is amazing, it used to be more confusing but I forgot about after the sink. Thanks so much for making this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL takewhile
I'm confused, shouldn't reassignment nodes always be the vulnerability chain (i.e. path from the source to the sink)? https://github.com/python-security/pyt/blob/master/pyt/vulnerabilities/vulnerabilities.py#L460 always gets hit, because I made --trim always true in https://github.com/python-security/pyt/blob/master/pyt/usage.py#L113-L118, but never cleaned up the UImode.NORMAL references e.g. https://github.com/python-security/pyt/blob/master/pyt/__main__.py#L68-L72 and https://github.com/python-security/pyt/blob/master/pyt/vulnerabilities/vulnerabilities.py#L459
I totally agree with you about the output.
You're absolutely right. I'm stupid. The tests default to UImode.NORMAL but actually TRIM does what I want. Everything already works fine. 🤦♂️
You're definitely not stupid 🙂
I should still open and merge, it'll make it easier to clean up the UImode.NORMAL stuff later on :) The code is confusing because of it.
OK 😄
The output should consist of the path from the source to the sink.
Anything which happens after the source reaches the sink is irrelevant
and just makes the output longer and confusing to interpret.
None of the lines removed from the tests actually affected the
vulnerability chain.
Perhaps this should be dealt with somewhere in the definition_chain or
vulnerability functions: here we just trim the chain upon reaching the
sink in the vulnerability_helper.