Skip to content

Conversation

RasmusWL
Copy link
Member

I think this is a great starting point for modeling outgoing HTTP client requests. I wouldn't say it's perfect, but it should be good enough to get me going for the SSRF query I'm trying to get finished 😄

@RasmusWL RasmusWL requested a review from a team as a code owner February 24, 2020 13:59
@RasmusWL RasmusWL force-pushed the python-http-clients branch from a976a07 to d268cbc Compare February 25, 2020 13:21
Copy link
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

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

A few comments, otherwise LGTM.

call_origin.getObject().pointsTo(_, constructor_call_value, constructor_call) and
cls = constructor_call_value.getClass() and
constructor_call = cls.getACall()
)
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 This looks quite complicated. It's not immediately clear to be why the last two lines are needed. Do you get spurious instances if you omit those lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I should have left a comment in there explaining why I needed to do that.

Taus poitned out that the reuqest being send off, doesn't *need* to happen on a
CallNode. Someone *could* use a __setattr__ or property :\
Move the stdlib tests from test/{2,3}/library-tests/ into /test/library-tests/,
and deal with version by using sys.version_info (results should be the same for
both versions).

six tests were moved from /library-tests/web/client/stdlib => /library-tests/web/client/six
@RasmusWL RasmusWL force-pushed the python-http-clients branch from d517aca to 4330d4e Compare February 26, 2020 09:27
@tausbn tausbn merged commit 85f5ad2 into github:master Feb 26, 2020
@RasmusWL RasmusWL deleted the python-http-clients branch February 27, 2020 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants