[issue19094] urljoin should raise a TypeError if URL is not a string

[Jacob Walls]

Change by Jacob Walls :


--
pull_requests: +25276
pull_request: https://github.com/python/cpython/pull/26687

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Irit Katriel]

Change by Irit Katriel :


--
versions: +Python 3.11 -Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Jacob Walls]

Jacob Walls  added the comment:

Hi vajrasky, do you have any interest in converting your patch to a GitHub PR? 
If not I can see about doing so myself. Cheers.

--
nosy: +jacobtylerwalls

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Irit Katriel]

Irit Katriel  added the comment:

Still broken in 3.10:

Running Release|x64 interpreter...
Python 3.10.0a5+ (heads/master:bf2e7e55d7, Feb 11 2021, 23:09:25) [MSC v.1928 
64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import urllib.parse
>>> urllib.parse.urljoin('foo', [])
'foo'
>>>

--
nosy: +iritkatriel
versions: +Python 3.10, Python 3.8, Python 3.9 -Python 2.7, Python 3.4, Python 
3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

About acceptable behavior with wrong arguments types see discussions in 
issue22766 and http://comments.gmane.org/gmane.comp.python.devel/150073 .

--
nosy: +serhiy.storchaka
versions: +Python 3.5 -Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Vajrasky Kok]

Vajrasky Kok added the comment:

Okay, attached the patch based on your comment, Senthil Kumaran. Thanks.

The reason I use isinstance to check the type is because I want to support the 
inheritance as much as possible.

 class new_str(str):
...   pass
 urljoin(new_str('http://python.org') + new_str('hehe'))
will not work with the newest patch.

Also, in Lib/urllib/parse.py, most of the time, we use isinstance(x, str) not 
type(x) == str.

But I don't know. Maybe it does not matter.

--
Added file: http://bugs.python.org/file31933/urljoin_throws_type_error_v3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Jason R. Coombs]

Jason R. Coombs added the comment:

Vajrasky, you're right. Comparing against type(obj) is an anti-pattern. 
isinstance is better. Duck typing is even better (in many cases).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Senthil Kumaran]

Senthil Kumaran added the comment:

I have provided my comments in the review tool. Please check them out.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Vajrasky Kok]

Vajrasky Kok added the comment:

I modified the patch to handle the last case using your way as well.

Anyway, I found out that urlsplit and urlparse got the same issue as well.

 urlparse('python.org', b'http://')
Traceback (most recent call last):
  File stdin, line 1, in module
  File 
/home/sky/Code/python/programming_language/cpython/Lib/urllib/parse.py, line 
292, in urlparse
url, scheme, _coerce_result = _coerce_args(url, scheme)
  File 
/home/sky/Code/python/programming_language/cpython/Lib/urllib/parse.py, line 
109, in _coerce_args
raise TypeError(Cannot mix str and non-str arguments)
TypeError: Cannot mix str and non-str arguments
 urlparse('python.org', b'')
ParseResult(scheme=b'', netloc='', path='python.org', params='', query='', 
fragment='')
 urlparse('python.org', 0)
ParseResult(scheme=0, netloc='', path='python.org', params='', query='', 
fragment='')

Same thing happens in urlsplit. Fortunately, urlunsplit and urlunparse don't 
have this issue.

--
Added file: http://bugs.python.org/file31889/urljoin_throws_type_error_v2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
nosy: +orsenthil
stage:  - patch review
type:  - behavior

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Jason R. Coombs]

Jason R. Coombs added the comment:

I don't mind the AttributeError - that's a reasonable exception when passing 
invalid types in, and that's in fact the current behavior. The example of 
(['a'], []) does bother me though. Those inputs are also seemingly invalid, 
though somewhat more compatible from a 'urljoin' perspective than two different 
types.

You've given me reason to re-consider your patch.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Berker Peksag]

Changes by Berker Peksag berker.pek...@gmail.com:


--
nosy: +berker.peksag

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Vajrasky Kok]

Vajrasky Kok added the comment:

This is the preliminary patch to raise TypeError in that situation.

--
keywords: +patch
nosy: +vajrasky
Added file: http://bugs.python.org/file31876/urljoin_throws_type_error.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Jason R. Coombs]

Jason R. Coombs added the comment:

Thanks Vajraski for the patch (especially the tests).

A colleague reminded me of an aphorism by Raymond Hettinger from the recent 
PyCon paraphrased: duck typing is superior to isinstance.

Maybe instead consider something like this:

diff -r 7f13d5ecf71f Lib/urllib/parse.py
--- a/Lib/urllib/parse.py   Sun Sep 22 09:33:45 2013 -0400
+++ b/Lib/urllib/parse.py   Thu Sep 26 18:52:18 2013 -0400
@@ -405,10 +405,9 @@
 def urljoin(base, url, allow_fragments=True):
 Join a base URL and a possibly relative URL to form an absolute
 interpretation of the latter.
-if not base:
-return url
-if not url:
-return base
+if not base or not url:
+# one of the inputs is empty, so simply concatenate
+return base + url
 base, url, _coerce_result = _coerce_args(base, url)
 bscheme, bnetloc, bpath, bparams, bquery, bfragment = \
 urlparse(base, '', allow_fragments)

This implementation is not only shorter and raises TypeErrors if the types 
aren't compatible, but those errors are triggered by the underlying 
implementation. Furthermore, if either the url or base were to be a duck-type 
that met the necessary interface, it need not be a string subclass.

I don't feel strongly either way, but I'm slightly more inclined to accept the 
simpler implementation. I'm interested in the everyone's thoughts on either 
approach.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Vajrasky Kok]

Vajrasky Kok added the comment:

What about this one?

 urljoin(['a'], [])
['a']
 urljoin(['a'], ['b'])
. omitted ..
AttributeError: 'list' object has no attribute 'decode'

Is this desirable? Both patches missed this case. Should we only accept str and 
bytes?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19094] urljoin should raise a TypeError if URL is not a string

[Jason R. Coombs]

New submission from Jason R. Coombs:

 import urllib.parse
 urllib.parse.urljoin('foo', [])
'foo'

That should raise a TypeError, not succeed quietly as if an empty string were 
passed.

--
components: Library (Lib)
messages: 198418
nosy: jason.coombs
priority: normal
severity: normal
status: open
title: urljoin should raise a TypeError if URL is not a string
versions: Python 2.7, Python 3.3, Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19094
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com