Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed url checker ':' to 'http' #4678

Merged
merged 1 commit into from Feb 1, 2017

Conversation

iashraful
Copy link
Contributor

@iashraful iashraful commented Nov 14, 2016

Description

RequestClient In this line it's checking with ':'. But think when url="example.com:2121" then this this check can't be OK. So if it checks only for http or https then we can use http for checking. :) That's all. 馃憤

@@ -114,7 +114,7 @@ def __init__(self, *args, **kwargs):
self.mount('https://', adapter)

def request(self, method, url, *args, **kwargs):
if ':' not in url:
if 'http' not in url:
Copy link
Contributor

@ticosax ticosax Nov 14, 2016

Choose a reason for hiding this comment

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

if not url.startswith('http') would be even better.

Copy link
Contributor Author

@iashraful iashraful Nov 14, 2016

Choose a reason for hiding this comment

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

@ticosax Yes. It can be. So, should I update PR ??

Copy link
Member

@tomchristie tomchristie Nov 14, 2016

Choose a reason for hiding this comment

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

Yup

@@ -114,7 +114,7 @@ def __init__(self, *args, **kwargs):
self.mount('https://', adapter)

def request(self, method, url, *args, **kwargs):
if ':' not in url:
if 'http' not in url:
Copy link
Member

@tomchristie tomchristie Nov 14, 2016

Choose a reason for hiding this comment

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

Yup

@tomchristie tomchristie merged commit 2ec3db8 into encode:master Feb 1, 2017
3 checks passed
@tomchristie tomchristie added this to the 3.5.4 Release milestone Feb 1, 2017
vpistis added a commit to vpistis/django-rest-framework that referenced this pull request Feb 6, 2017
* master: (95 commits)
  Add max_length and min_length arguments for ListField (encode#4877)
  Update sponsors on README to include rollbar (encode#4876)
  Added Rollbar to premium sponsors
  fixed url checker ':' to 'http' (encode#4678)
  Fixed `dedent` for tab indent.
  Add test for pagination when limit not set
  Fixed Django 2.0 compatibility due to `django.conf.urls.include` parameters change. (encode#4866)
  Add remaining tests for generics (encode#4865)
  Removed unnecessary importlib wrapper.
  Updated Support section and added funding email (encode#4860)
  Git add remaining tests for BaseSerializer (encode#4857)
  Update machinalis links
  Updated Machinalis links (encode#4856)
  Add remaining tests for metadata (encode#4855)
  Fixed encode#4574 -- Removed multi-table inheritance auto created PK from serialize. (encode#4852)
  Add couple of tests for filters (encode#4849)
  Refactor according to comments in PR (encode#4848)
  Add couple of tests for compat module (encode#4845)
  Direct readers to info on how to add viewset docs (encode#4844)
  Fixed test_one_to_one_with_inheritance. Added required on_delete attribute to OneToOneField field. (encode#4838)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants