Fix #19294, Ruby NetHttpRequest improvements #20101
Conversation
| * | ||
| * # connection re-use | ||
| * Net::HTTP.start("http://example.com") do |http| | ||
| * http.get("/") | ||
| * end |
There was a problem hiding this comment.
Note that technically Net::HTTP.new does not open/reuse connections/sessions: https://docs.ruby-lang.org/en/master/Net/HTTP.html#method-c-new
| http = Net::HTTP.new("https://example.com") | ||
| root_get = Net::HTTP::Get.new("/") | ||
| http.request(root_get) |
There was a problem hiding this comment.
This case was already detected without the above code modifications, but I wanted to make sure that request objects are also detected correctly.
| @@ -30,20 +37,27 @@ class NetHttpRequest extends Http::Client::Request::Range instanceof DataFlow::C | |||
| | | |||
| // Net::HTTP.get(...) | |||
| method in ["get", "get_response"] and | |||
| requestNode = API::getTopLevelMember("Net").getMember("HTTP").getReturn(method) and | |||
| connectionNode = API::getTopLevelMember("Net").getMember("HTTP") and | |||
| requestNode = connectionNode.getReturn(method) and | |||
There was a problem hiding this comment.
You can move requestNode = connectionNode.getReturn(method) up after line 36, and then remove the three duplicated lines.
There was a problem hiding this comment.
It's curious to me that you can set requestNode before you've set connectionNode in this situation. I've made the requested change, but I'd like to learn more. Is there some documentation you can point me to describing this behavior, or is that "just the way it works" with queries?
There was a problem hiding this comment.
QL is a logic language, so you should not think of x = y as an assignment, but rather as a constraint (x must be equal to y). So the order doesn't really matter.
| * ``` | ||
| */ | ||
| class NetHttpRequest extends Http::Client::Request::Range instanceof DataFlow::CallNode { | ||
| private DataFlow::CallNode request; | ||
| private API::Node requestNode; | ||
| API::Node requestNode; | ||
| API::Node connectionNode; |
Check notice
Code scanning / CodeQL
Field only used in CharPred Note
This PR makes 3 improvements to the
NetHttpRequestclass:connectionNodelike other Ruby HTTP clients (see Ruby NetHttpRequest improvements #19294)requestNodeandconnectionNodepublic so subclasses can use them (see Ruby NetHttpRequest improvements #19294)Net::HTTP.start- this is a common way to make HTTP requests in Ruby