Skip to content

Ruby: generalise summaries for ActiveSupport Hash extensions #11166

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

Merged
merged 7 commits into from
Nov 10, 2022

Conversation

nickrolfe
Copy link
Contributor

No description provided.

@nickrolfe nickrolfe added the Ruby label Nov 8, 2022
@nickrolfe nickrolfe requested a review from a team as a code owner November 8, 2022 16:03
ReverseMergeSummary() { this = ["reverse_merge", "with_defaults"] }

override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = ["Argument[self]", "Argument[0]"] and
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we missing a .WithElement here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. After some thought I've put the old summaries back (the ones that are specific to the context of ActionController params), and then made these general summaries use WithElement.

ReverseMergeBangSummary() { this = ["reverse_merge!", "with_defaults!", "reverse_update"] }

override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = ["Argument[self]", "Argument[0]"] and
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

- revert the changes to the taint summaries specific to ActionController
  params
- make the general flow summaries value-preserving and use
  WithElement[any]
hvitved
hvitved previously approved these changes Nov 9, 2022
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Let's see what DCA says.

@nickrolfe
Copy link
Contributor Author

DCA shows a bad row under stage timings. I don't really know what to make of that.

But I need to resolve conflicts anyway, so I'll kick off another run after that.

@nickrolfe nickrolfe merged commit 2f9f1f7 into main Nov 10, 2022
@nickrolfe nickrolfe deleted the nickrolfe/active_support_flow_summaries branch November 10, 2022 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants