urljoin duplicate slashes

[demianbrecht]

BPO	22278
Nosy	@orsenthil, @pitrou, @scoder, @vadmium, @demianbrecht
Files	issue22278.patch issue22278_2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/orsenthil'
closed_at = <Date 2014-09-22.07:50:02.643>
created_at = <Date 2014-08-26.17:58:07.218>
labels = ['type-bug', 'library']
title = 'urljoin duplicate slashes'
updated_at = <Date 2015-04-15.23:11:15.022>
user = 'https://github.com/demianbrecht'

bugs.python.org fields:

activity = <Date 2015-04-15.23:11:15.022>
actor = 'berker.peksag'
assignee = 'orsenthil'
closed = True
closed_date = <Date 2014-09-22.07:50:02.643>
closer = 'orsenthil'
components = ['Library (Lib)']
creation = <Date 2014-08-26.17:58:07.218>
creator = 'demian.brecht'
dependencies = []
files = ['36479', '36652']
hgrepos = []
issue_num = 22278
keywords = ['patch']
message_count = 11.0
messages = ['225923', '226164', '226168', '226240', '226249', '227048', '227086', '227254', '227255', '227273', '238494']
nosy_count = 6.0
nosy_names = ['orsenthil', 'pitrou', 'scoder', 'python-dev', 'martin.panter', 'demian.brecht']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22278'
versions = ['Python 3.5']

[demianbrecht]

Reported by Stefan Behnel in bpo-22118:

I'm now getting duplicated slashes in URLs, e.g.:

https://new//foo.html
http://my.little.server/url//logo.gif

In both cases, the base URL that gets joined with the postfix had a trailing slash, e.g.

"http://my.little.server/url/" + "logo.gif" -> "http://my.little.server/url//logo.gif"

[pitrou]

This patch seems to fix issue bpo-22311 as well (*). However it would be good to add more tests for base URLs with trailing slashes, it seems.

(*) without patch:

>>> base = """https://pypi.python.org/simple/werkzeug/"""
>>> rel = """../../packages/2.3/W/Werkzeug/Werkzeug-0.3.1-py2.3.egg#md5=5f669acf04af135ad8577d99a4387504"""
>>> urllib.parse.urljoin(base, rel)
'https://pypi.python.org/simple/packages/2.3/W/Werkzeug/Werkzeug-0.3.1-py2.3.egg#md5=5f669acf04af135ad8577d99a4387504'

with patch:

>>> urllib.parse.urljoin(base, rel)
'https://pypi.python.org/packages/2.3/W/Werkzeug/Werkzeug-0.3.1-py2.3.egg#md5=5f669acf04af135ad8577d99a4387504'

[scoder]

Were the tests in

http://bugs.python.org/file32591/urischemes.py

merged yet, that Nick Coghlan mentioned in http://bugs.python.org/issue22118#msg225662 ?

[pitrou]

Those tests don't seem to bring much. Part of them are straight from the RFC (and therefore already in the current test suite, I assume), part of them are for non-HTTP protocols such as "fred" (!). A couple of them seem to be genuine, although only one fails and it's a corner case.

[demianbrecht]

I'll try to get some time this week to extend the various test cases, thanks for pointing that out Antoine.

I also found that, other than the few RFC-specific blocks in the link that Nick added in the other ticket, not only were they questionable (non-HTTP as Antoine pointed out), but they were also just plain wrong in some cases given the new semantics.

[demianbrecht]

Antoine: On (finally) getting back to this and re-reading your test case, the current behaviour is incorrect and is corrected by the patch. I've added a few more test cases to ensure trailing slashes are handled correctly.

[orsenthil]

Except for the minor comments made by Antoine in the review, the patch looks good to go.

[python-dev]

New changeset 901e4e52b20a by Senthil Kumaran in branch 'default':
Issue bpo-22278: Fix urljoin problem with relative urls, a regression observed
https://hg.python.org/cpython/rev/901e4e52b20a

[orsenthil]

I addressed Antoine's comments with the patch and committed it. Thank you!

[demianbrecht]

Heh, I'd finally gotten a few minutes to address the comments... And it's already taken care of ;) Thanks Senthil.

[vadmium]

I opened bpo-23703 for a regression caused by this commit.