urllib.parse.urljoin does not handle WebSocket URLs

[imrehg]

BPO	25895
Nosy	@rbtcollins, @bitdancer, @berkerpeksag, @vadmium, @MarkusH
PRs	[Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
Files	0001-urllib.parse-enable-WebSocket-URL-schemes-to-be-corr.patch 0001-Enable-WebSocket-URL-schemes-in-urllib.parse.urljoin.patch 0001-Enable-WebSocket-URL-schemes-in-urllib.parse.urljoin.patch 0001-Enable-WebSocket-URL-schemes-in-urllib.parse.urljoin.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 = None
closed_at = <Date 2016-09-16.11:46:51.293>
created_at = <Date 2015-12-17.05:58:53.616>
labels = ['3.7', 'type-bug', 'library']
title = 'urllib.parse.urljoin does not handle WebSocket URLs'
updated_at = <Date 2017-03-31.16:36:12.867>
user = 'https://bugs.python.org/imrehg'

bugs.python.org fields:

activity = <Date 2017-03-31.16:36:12.867>
actor = 'dstufft'
assignee = 'none'
closed = True
closed_date = <Date 2016-09-16.11:46:51.293>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2015-12-17.05:58:53.616>
creator = 'imrehg'
dependencies = []
files = ['41334', '44612', '44615', '44649']
hgrepos = []
issue_num = 25895
keywords = ['patch']
message_count = 17.0
messages = ['256581', '256582', '256583', '256587', '256628', '271722', '276126', '276130', '276142', '276152', '276155', '276160', '276163', '276292', '276379', '276701', '276702']
nosy_count = 7.0
nosy_names = ['rbcollins', 'r.david.murray', 'python-dev', 'berker.peksag', 'martin.panter', 'MarkusH', 'imrehg']
pr_nums = ['552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25895'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[imrehg]

Handling the "ws" and "wss" schemes of the WebSocket protocol in urllib.parse.urlparse was discussed in a previous issue http://bugs.python.org/issue13244 Parsing them seems to work correctly:

>>> urllib.parse.urlparse("wss://domain/endpoint")
ParseResult(scheme='wss', netloc='domain', path='/endpoint', params='', query='', fragment='')

On the other hand, urllib.parse.urljoin does not know about these schemes and thus cannot be used to construct WebSocket URLs (as, for example some streaming data APIs now seem to use it, such as https://starfighter.readme.io/docs/quotes-ticker-tape-websocket).

Using the example above I get:

>>> urllib.parse.urljoin("wss://domain/","/endpoint")
'/endpoint'

This is not the expected result, 'wss://example.com/endpoint', because these WebSocket schemes are not included in the uses_relative and uses_netloc lists in the library settings. For clarity, based on the previous issue's discussion and RFC6455 Section 11.1 https://tools.ietf.org/html/rfc6455#section-11.1 , these are the only two portions that apply, and should not be listed in uses_params.

As a workaround, similarly to the previous issue's recommendation, one can use:

> import urlparse.parse
> wsschemes = ["ws", "wss"]
> urlparse.parse.uses_relative.extend(wsschemes)
> urlparse.parse.uses_netloc.extend(wsschemes)

Attached patch to this effect. Also added tests to cover these schemes, though comments are definitely welcome on all of this!

[1st1]

Gergely, the patch looks alright. Please sign the contributor agreement and we'll have it merged.

[imrehg]

Just signed it, thanks!

[vadmium]

The documentation should probably also be updated. It has a list of supported schemes. I imagine this would have to be taken as a new feature for 3.6+, rather than a bug fix.

An alternative would be to enable joining for arbitrary schemes; see bpo-18828.

[1st1]

The documentation should probably also be updated. It has a list of supported schemes.

I agree. Gergely, do you want to update your patch?

I imagine this would have to be taken as a new feature for 3.6+, rather than a bug fix.

Since urlparse can parse wss:// addresses, *but* urljoin can't work with them, I think we can classify this as a bug fix patch.

[vadmium]

My view is that because the schemes are not in the documented list of supported schemes, this is a new feature, and any documentation update should include a “versionadded” or similar notice.

[rbtcollins]

It is a bugfix, but we should document when it started working using the version added segment IMO. I think addding this to 3.6 now would be ok.

[MarkusH]

As discussed with rbcollins during the KiwiPyCon sprints, I'll add patches for 3.5, 3.6 and master with docs mentioning the addition of ws and wss as of 3.5.3

[MarkusH]

Since the patch applies cleanly to the 3.5, 3.6 and master branch I only attached one updated version of it.

[berkerpeksag]

I think a Misc/NEWS entry is enough. We didn't use versionadded/versionchanged directives for similar changes in the past (mimetypes.types_map for example) Even if we decide to add an annotation it should be versionchanged, not versionadded (the API is not new.)

The rest of the patch looks good to me.

[MarkusH]

Thanks for your input, Berker. Updated as suggested. I still include the versionchanged annotation as I suspect more people to look at the docs than the entire changelog

[rbtcollins]

I find details like this extremely useful in the main docs, so please do add there.