-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Python: Model CookieWrites from HeaderWrites #16696
Python: Model CookieWrites from HeaderWrites #16696
Conversation
8a1998e
to
0901b3d
Compare
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.
The only two things I wanted to point out was fixed later on, so LGTM 👍
{ | ||
CookieHeaderWrite() { | ||
exists(StringLiteral str | | ||
str.getText() = "Set-Cookie" and |
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.
should this be case sensitive? I see this was fixed later
override DataFlow::Node getNameArg() { | ||
result = this.(Http::Server::ResponseHeaderWrite).getValueArg() | ||
} |
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.
When I see this was fixed later.getHeaderArg
is implemented, I would expect both name+value to be none()
.
Part of https://github.com/github/codeql-python-team/issues/792 promoting #6360; as well as a follow-up to #16105
This PR defines new instances of the
CookieWrite
concept in terms of theHeaderWrite
concept; as is done in experimental with theCookie
concept.