Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Conversation

@max-schaefer
Copy link
Contributor

cf #108

cc @porcupineyhairs

@max-schaefer max-schaefer requested a review from sauyon May 4, 2020 08:15
Copy link
Contributor

@sauyon sauyon left a comment

Choose a reason for hiding this comment

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

Two minor comments, otherwise LGTM.

override DataFlow::Node getData() { result = this.getArgument([1, 3, 4]) }
}
/** Gets the package name `github.com/sendgrid/sendgrid-go/helpers/mail`. */
bindingset[result]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be removed, or should the body be changed to use the package versioning predicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do you think it should be removed? It's essentially just a symbolic constant because the string is used twice.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was specifically talking about the bindingset, I should have been more clear.

Or is there something about the annotation I don't understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're absolutely right about that. The bindingset is spurious. Let me remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Co-authored-by: Sauyon Lee <sauyon@github.com>
@max-schaefer max-schaefer force-pushed the clean-up-email-injection branch from 8d06957 to 54f1015 Compare May 5, 2020 10:24
@max-schaefer max-schaefer merged commit ca0d9cc into github:master May 5, 2020
@max-schaefer max-schaefer deleted the clean-up-email-injection branch August 28, 2020 06:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants