urllib.parse.quote safe Parameter Not Optional

[willayd]

BPO	35041
Nosy	@ammaraskar, @WillAyd

^{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 2018-10-22.02:37:11.075>
created_at = <Date 2018-10-22.02:25:11.414>
labels = ['3.7', 'invalid']
title = 'urllib.parse.quote safe Parameter Not Optional'
updated_at = <Date 2018-10-22.04:25:23.756>
user = 'https://github.com/willayd'

bugs.python.org fields:

activity = <Date 2018-10-22.04:25:23.756>
actor = 'william.ayd'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-22.02:37:11.075>
closer = 'ammar2'
components = []
creation = <Date 2018-10-22.02:25:11.414>
creator = 'william.ayd'
dependencies = []
files = []
hgrepos = []
issue_num = 35041
keywords = []
message_count = 8.0
messages = ['328229', '328230', '328231', '328232', '328233', '328234', '328235', '328236']
nosy_count = 2.0
nosy_names = ['ammar2', 'william.ayd']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue35041'
versions = ['Python 3.7']

[willayd]

The safe parameter in urllib.parse.quote is documented as optional. However, the following will raise TypeError: 'NoneType' object is not iterable:

urllib.parse.quote("/", safe=None)

whereas explicitly providing an iterable will allow the function to succeed:

urllib.parse.quote("/", safe=[])

[ammaraskar]

The documentation also goes on to say that its default value is '/'. It is optional in the sense that the function can be called without providing it.

It doesn't mean that None is somehow a valid type for it. Consider the open function for example: https://docs.python.org/3/library/functions.html#open

The documentation says that, "mode is an optional string that specifies the mode in which the file is opened. It defaults to 'r' "

This doesn't mean you can just go off and do open("file.txt", None)

[willayd]

Semantics aside is it still the intended behavior that these calls should work:

urllib.parse.quote("/", safe='')

AND

urllib.parse.quote("/", safe=[])

But that this should raise?

urllib.parse.quote("/", safe=None)

IMO seems counterintuitive

[ammaraskar]

I would say so since the documentation says: safe parameter specifies additional ASCII characters

None isn't really a set of ASCII characters but the empty string and empty list are.

[ammaraskar]

Does rewording that prior part to "safe parameter specifies an iterable of additional ASCII characters" make it less confusing?

[willayd]

Hmm well I still personally feel that the implementation is somewhat off mores than the documentation. Specifically I think it is confusing that it accepts an empty iterable but not one containing elements.

This is fine:

urllib.parse.quote("/", safe=[])

Though this isn't:

urllib.parse.quote("/", safe=['/'])

Even though the following two calls are fine (though with different return values as expected):

urllib.parse.quote("/", safe='')
urllib.parse.quote("/", safe='/')

It might go against the spirit of duck typing but I find it very nuanced that empty iterables are allowed but if non-empty it must be a string. Would it not make more sense to raise if a non-String type is passed?

[ammaraskar]

I agree that

urllib.parse.quote("/", safe=['/'])

should probably work. It looks like the reason it doesn't is because quote tries to normalize the safe iterable to ASCII characters only. As part of this it attempts to run < 128 on each element of safe.

As part of this it does

'/' < 128

and fails.

[willayd]

What if we instead just raised for anything that isn't a string or a byte? The docstring for quote suggests that it should only accept str or byte objects for safe, though it doesn't enforce that:

cpython/Lib/urllib/parse.py

Line 791 in 121eb16

string and safe may be either str or bytes objects. encoding and errors

Seems like it would be easier to enforce that rather than trying to accept any arbitrary iterable.