Add support for ipv6 address format when parsing url [#648]#649
Conversation
| def get_url_dict(url): | ||
| scheme, netloc, path, params, query, fragment = compat.urlparse.urlparse(url) | ||
| if ":" in netloc: | ||
| if '[' in netloc and ']' in netloc: |
There was a problem hiding this comment.
I had a quick look at ParseResult (the object that urlparse returns). It has properties for hostname and port, which properly support IPv6 URLs since Python 2.7. Only drawback I see is that it'll lowercase IPv6 addresses, but that seems like it isn't a huge problem.
So we would do something like this:
parse_result = compat.urlparse.urlparse(url)
And then use parse_result.hostname, parse_result.port, parse_result.scheme etc. What do you think?
There was a problem hiding this comment.
@beniwohli You're right. I refactored the code to use ParseResult. Since the port is returned as integer, I had to add small hack to map into string, so not to introduce any regressions. Thanks for the help.
|
jenkins run the tests please |
beniwohli
left a comment
beniwohli
left a comment
There was a problem hiding this comment.
Awesome work, thank you so much @mariodev! Just a small little nitpick from our code formatter :)
We have all our linters and code formatters also available as pre-commit hooks if you'd like to send us more pull requests in the future :)
also, use proper parametrization for test_get_url_dict test cases
|
@mariodev I shipped this in version 5.3.2 of the agent. Thanks again for reporting and fixing the bug! |
3 participants
What does this pull request do?
The changes are improving
get_url_dictutil function, to correctly parse ipv6 address.Why is it important?
Allows to handle request without throwing exception. More details in related issue.
Related issues
closes #648