Issue 9873: urllib.parse: Allow bytes in some APIs that use string literals internally (original) (raw)
Created on 2010-09-16 13:25 by ncoghlan, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Messages (29)
Author: Alyssa Coghlan (ncoghlan) * 
Date: 2010-09-16 13:25
As per python-dev discussion in June, many Py3k APIs currently gratuitously prevent the use of bytes and bytearray objects as arguments due to their use of string literals internally.
Examples: urllib.parse.urlparse urllib.parse.urlunparse urllib.parse.urljoin urllib.parse.urlsplit (and plenty of other examples in urllib.parse)
While a strict reading of the relevant RFCs suggests that strings are the more appropriate type for these APIs, as a practical matter, protocol developers want to be able to operate on ASCII supersets as raw bytes.
The proposal is to modify the implementation of these functions such that string literals are used only with string arguments, and bytes literals otherwise. If a function accepts multiple values and a mixture of strings and bytes is passed in then the operation will still fail (as it should).
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-17 13:43
From the python-dev thread (http://mail.python.org/pipermail/python-dev/2010-September/103780.html):
So the domain of any polymorphic text manipulation functions we define would be:
- Unicode strings
- byte sequences where the encoding is either:
- a single byte ASCII superset (e.g. iso-8859-, cp1252, koi8, mac*)
- an ASCII compatible multibyte encoding (e.g. UTF-8, EUC-JP)
Passing in byte sequences that are encoded using an ASCII incompatible multibyte encoding (e.g. CP932, UTF-7, UTF-16, UTF-32, shift-JIS, big5, iso-2022-*, EUC-CN/KR/TW) or a single byte encoding that is not an ASCII superset (e.g. EBCDIC) will have undefined results.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-20 11:42
The design approach (at least for urllib.parse) is to add separate *b APIs that operate on bytes rather than implicitly allowing bytes in the ordinary versions of the function.
Allowing programmers to manipulate correctly encoded (and hence ASCII compatible) bytes to avoid decode/encode overhead when manipulating URLs is fine (and the whole point of adding the new functions). Allowing them to not know whether they have encoded data or text suitable for display to the user isn't necessary (and is easy to add later if we decide we want it, while taking it away is far more difficult).
More detail at http://mail.python.org/pipermail/python-dev/2010-September/103828.html
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-28 14:00
Attached patch is a very rough first cut at this. I've gone with the basic approach of simply assigning the literals to local variables in each function that uses them. My rationale for that is:
- Every function has to have some kind of boilerplate to switch based on the type of the argument
- Some functions need to switch other values (such as the list of scheme_chars or the unparse function), so a separate object with literal attributes won't be enough
- Given 1 and 2, the overhead of a separate namespace for the literal references isn't justified
I've also gone with a philosophy that only str objects are treated as strings and everything else is implicitly assumed to be bytes. This is in keeping with the general interpreter behaviour where we really don't support duck-typing when it comes to strings.
The test updates aren't comprehensive yet, but they were enough to uncover quite a few things I had missed.
quoting is still a bit ugly, so I may still add a byte->bytes/str->str variant of those APIs.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-28 21:56
A possible duck-typing approach here would be to replace the "instance(x, str)" tests with "hasattr(x, 'encode')" checks instead.
Thoughts?
Author: Antoine Pitrou (pitrou) *
Date: 2010-09-29 09:57
A possible duck-typing approach here would be to replace the "instance(x, str)" tests with "hasattr(x, 'encode')" checks instead.
Looks more ugly than useful to me. People wanting to emulate str had better subclass it anyway...
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-29 20:49
Agreed - I think there's a non-zero chance of triggering the str-path by mistake if we try to duck-type it (I just added a similar comment to #9969 regarding a possible convenience API for tokenisation)
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-29 20:56
Added to Reitveld: http://codereview.appspot.com/2318041/
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-29 21:55
One of Antoine's review comments made me realise I hadn't explicitly noted the "why not decode with latin-1?" rationale for the bytes handling. (It did come up in one or more of the myriad python-dev discussions on the topic, I just hadn't noted it here)
The primary reason for supporting ASCII compatible bytes directly is specifically to avoid the encoding and decoding overhead associated with the translation to unicode text. Making that overhead implicit rather than avoiding it altogether would be to quite miss the point of the API change.
Author: Antoine Pitrou (pitrou) *
Date: 2010-09-29 22:09
The primary reason for supporting ASCII compatible bytes directly is specifically to avoid the encoding and decoding overhead associated with the translation to unicode text.
I think it's quite misguided. latin1 encoding and decoding is blindingly fast (on the order of 1GB/s. here). Unless you have multi-megabyte URLs, you won't notice any overhead.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-29 22:23
I think it's quite misguided. latin1 encoding and decoding is blindingly fast (on the order of 1GB/s. here). Unless you have multi-megabyte URLs, you won't notice any overhead.
Ah, I didn't know that (although it makes sense now I think about it). I'll start exploring ideas along those lines then. Having to name all the literals as I do in the patch is really quite ugly.
A general sketch of such a strategy would be to stick the following near the start of affected functions:
encode_result = not isinstance(url, str) # or whatever the main parameter is called if encode_result: url = url.decode('latin-1') # decode any other arguments that need it # Select the bytes versions of any relevant globals else: # Select the str versions of any relevant globals
Then, at the end, do an encoding step. However, the encoding step may get a little messy when it comes to the structured data types. For that, I'll probably take a leaf out of the email6 book and create a parallel bytes API, with appropriate encode/decode methods to transform one into the other.
Author: STINNER Victor (vstinner) *
Date: 2010-09-29 22:48
I don't understand why you would like to implicitly convert bytes to str (which is one of the worse design choice of Python2). If you don't want to care about encodings, use bytes is fine. Decode bytes using an arbitrary encoding is the fastest way to mojibake.
So You have two choices: create new functions with bytes as input and output (like os.getcwd() and os.getcwdb()), or the output type will depend on the input type (solution choosen by os.path). Example of ther later:
os.path.expanduser('
') '/home/haypo' os.path.expanduser(b'') b'/home/haypo'
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-30 10:54
From a function user perspective, the latter API (bytes->bytes, str->str) is exactly what I'm doing.
Antoine's point is that there are two ways to achieve that:
Option 1 (what my patch currently does):
- provide bytes and str variants of all constants
- choose which set to use at the start of each function
- be careful never to index, only slice (even for single characters)
- a few other traps that I don't remember off the top of my head
Option 2 (the alternative Antoine suggested and I'm considering):
- "decode" the ASCII compatible bytes to str objects by treating them as nominally latin-1
- use the same str-based constants as are used to handle actual str inputs
- be able to index to your heart's content inside the algorithm
- ensure that any bytes-as-pseudo-str objects are "encoded" back to actual bytes before they are returned
From outside the function, a user shouldn't be able to tell which approach we're using internally.
The nice thing about option 2 is to make sure you're doing it correctly, you only need to check three kinds of location:
- the initial parameter handling in each function
- any return statements, raise statements that allow a value to leave the function
- any yield expressions (both input and output)
The effects of option 1 are scattered all over your algorithms, so it's hard to be sure you've caught everything.
The downside of option 2 is if you make a mistake and let your bytes-as-pseudo-str objects escape from the confines of your function, you're going to see some very strange behaviour.
Author: STINNER Victor (vstinner) *
Date: 2010-09-30 10:59
Option 2 (the alternative Antoine suggested and I'm considering):
- "decode" ... to str ...
- ... objects are "encoded" back to actual bytes before they are returned
In this case, you have to be very careful to not mix str and bytes decoded to str using a pseudo-encoding. Dummy example: urljoin('unicode', b'bytes') should raise an error.
I don't care of the internals if you write tests to ensure that it is not possible to mix str and bytes with the public API.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-09-30 11:40
Yeah, the general implementation concept I'm thinking of going with for option 2 will use a few helper functions:
url, coerced_to_str = _coerce_to_str(url) if coerced_to_str: param = _force_to_str(param) # as appropriate ... return _undo_coercion(result, coerced_to_str)
The first helper function standardises the typecheck, the second one complains if it is given something that is already a string.
The last one just standardises the check to see if the coercion needs to be undone, and actually undoing the coercion.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-10-03 02:36
As per RDM's email to python-dev, a better way to create the pseudo_str values would be by decoding as ascii with a surrogate escape error handler rather than by decoding as latin-1.
Author: Antoine Pitrou (pitrou) *
Date: 2010-10-03 12:44
As per RDM's email to python-dev, a better way to create the pseudo_str values would be by decoding as ascii with a surrogate escape error handler rather than by decoding as latin-1.
If you were worried about performance, then surrogateescape is certainly much slower than latin1.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-10-04 12:01
Yeah, I'll have to time it to see how much difference latin-1 vs surrogateescape makes when the MSB is set in any bytes.
Author: STINNER Victor (vstinner) *
Date: 2010-10-05 07:32
If you were worried about performance, then surrogateescape is certainly much slower than latin1.
If you were really worried about performance, the bytes type is maybe faster than: decode bytes to str using latin-1, process str strings, encode str to bytes using latin-1.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-10-05 10:32
On Tue, Oct 5, 2010 at 5:32 PM, STINNER Victor <report@bugs.python.org> wrote:
STINNER Victor <victor.stinner@haypocalc.com> added the comment:
If you were worried about performance, then surrogateescape is certainly much slower than latin1.
If you were really worried about performance, the bytes type is maybe faster than: decode bytes to str using latin-1, process str strings, encode str to bytes using latin-1.
I'm fairly resigned to the fact that I'm going to need some kind of micro-benchmark to compare the different approaches. For example, the bytes based approach has a lot of extra assignments to local variables that the str based approach doesn't need.
The first step is to actually have a str-based patch to compare to the existing bytes based patch. If the code ends up significantly clearer (as I expect it will), we can probably sacrifice a certain amount of speed for that benefit.
Author: Senthil Kumaran (orsenthil) *
Date: 2010-10-05 11:01
I wonder if Option2 (ascii+surrogateescape vs latin1) is only about performance. How about escapes that might occur if the Option2 is adopted. That might take higher priority than performance. Do we know 'how tight' that approach is?
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-10-08 11:10
I've been pondering the idea of adopting a more conservative approach here, since there are actually two issues:
Properly quoted URLs are transferred as pure 7-bit ASCII (due to percent-encoding of everything else). However, most of the manipulation functions in urllib.parse can't handle bytes at all, even data that is 7-bit clean.
In the real world, just like email, URLs will often contain unescaped (or incorrectly escaped) characters. So assuming the input is actually pure ASCII isn't necessarily a valid assumption.
I'm wondering, since encoding (aside from quoting) isn't urllib.parse's problem, maybe what I should be looking at doing is just handling bytes input via an implicit ascii conversion in strict mode (and then conversion back when the processing is complete).
Then bytes manipulation of properly quoted URLs will "just work", while improperly quoted URLs will fail noisily. This isn't like email or http where the protocol contains encoding information that the library should be trying to interpret - we're just being given raw bytes without any context information.
If any application wants to be more permissive than that, it can do its own conversion to a string and then use the text-based processing. I'll add "encode" methods to the result objects to make it easy to convert their contents from str to bytes and vice-versa.
I'll factor out the implicit encoding/decoding such that if we decide to change the model later (ASCII-strict, ASCII-escape, latin-1) it shouldn't be too difficult.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-10-24 13:42
Attached a second version of the patch. Notable features:
- uses a coercion-to-str-and-back strategy (using ascii-strict)
- a significantly more comprehensive pass through the urlparse test suite. I'm happy that the test suite mods are complete with this pass.
The actual coercion-to-str technique I used standardises the type consistency check for the attributes and also returns a callable that handles the necessary coercion of any results. The parsed/split result objects gain encode/decode methods to allow that all to be handled polymorphically (although I think the decode methods may actually be redundant in practice).
There's a deliberate loophole in the type consistency checking to allow the empty string to be type-neutral. Without that, the scheme='' default argument to urlsplit and urlparse becomes painful (as do the urljoin shortcuts for base or url being the empty string).
Implementing this was night and day when compared to the initial attempt that tried to actually manipulate bytes input as bytes. With that patch, it took me multiple runs of the test suite to get everything working. This time, the only things I had to fix were typos and bugs in the additional test suite enhancements. The actual code logic for the type coercions worked first time.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-10-24 13:47
Unless I hear some reasonable objections within the next week or so, I'm going to document and commit the ascii-strict coercion approach for beta 1.
The difference in code clarity is such that I'm not even going to try to benchmark the two approaches against each other.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-11-07 12:17
Just a note for myself when I next update the patch: the 2-tuple returned by defrag needs to be turned into a real result type of its own, and the decode/encode methods on result objects should be tested explicitly.
Author: Éric Araujo (eric.araujo) *
Date: 2010-11-08 15:07
Related issue in .
Author: Hallvard B Furuseth (hfuru)
Date: 2010-11-09 10:37
urlunparse(url or params = bytes object) produces a result with the repr of the bytes object if params is set.
urllib.parse.urlunparse(['http', 'host', '/dir', b'params', '', '']) --> "http://host/dir;b'params'"
That's confusing since urllib/parse.py goes to a lot of trouble to support both bytes and str. Simplest fix is to only accept str:
Index: Lib/urllib/parse.py @@ -219,5 +219,5 @@ def urlunparse(components): scheme, netloc, url, params, query, fragment = components if params:
url = "%s;%s" % (url, params)
return urlunsplit((scheme, netloc, url, query, fragment))url = ';'.join((url, params))
Some people at comp.lang.python tell me code shouldn't anyway do str() just in case it is needed like urllib does, not that I can make much sense of that discussion. (Subject: harmful str(bytes)).
BTW, the str vs bytes code doesn't have to be quite as painful as in urllib.parse. Here is a patch which just rearranges and factors out some code. http://bugs.python.org/file19525/parse.diff
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-11-30 07:32
New patch which addresses my last two comments (i.e. some basic explicit tests of the encode/decode methods on result objects, and urldefrag returns a named tuple like urlsplit and urlparse already did).
A natural consequence of this patch is that mixed arguments (as in the message above) will be rejected with TypeError.
Once I figure out what the docs changes are going to look like, I'll wrap this all up and commit it.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2010-11-30 15:53
Committed in r86889
The docs changes should soon be live at: http://docs.python.org/dev/library/urllib.parse.html
If anyone would like to suggest changes to the wording of the docs for post beta1, or finds additional corner cases that the new bytes handling can't cope with, feel free to create a new issue.
History
Date
User
Action
Args
2022-04-11 14:57:06
admin
set
github: 54082
2010-11-30 15:53:40
ncoghlan
set
status: open -> closed
resolution: accepted
messages: +
stage: needs patch -> resolved
2010-11-30 07:32:09
ncoghlan
set
files: + issue9873_just_needs_docs.diff
messages: +
2010-11-09 10:37:27
hfuru
set
nosy: + hfuru
messages: +
2010-11-08 15:07:13
eric.araujo
set
messages: +
2010-11-07 12:17:38
ncoghlan
set
messages: +
2010-11-06 20:30:03
r.david.murray
link
2010-10-24 13:47:59
ncoghlan
set
messages: +
2010-10-24 13:42:51
ncoghlan
set
files: + issue9873_coercion_to_str.diff
messages: +
2010-10-08 18:22:04
sjt
set
nosy: + sjt
2010-10-08 11:10:41
ncoghlan
set
messages: +
2010-10-05 11:01:10
orsenthil
set
messages: +
2010-10-05 10:32:18
ncoghlan
set
messages: +
2010-10-05 07:32:03
vstinner
set
messages: +
2010-10-04 12:01:49
ncoghlan
set
messages: +
2010-10-03 12:44:49
pitrou
set
messages: +
2010-10-03 02:36:09
ncoghlan
set
messages: +
2010-10-01 23:04:26
vstinner
set
title: Allow bytes in some APIs that use string literals internally -> urllib.parse: Allow bytes in some APIs that use string literals internally
2010-09-30 11:57:36
orsenthil
set
nosy: + orsenthil
2010-09-30 11:40:33
ncoghlan
set
messages: +
2010-09-30 10:59:30
vstinner
set
messages: +
2010-09-30 10:54:31
ncoghlan
set
messages: +
2010-09-29 22:48:53
vstinner
set
nosy: + vstinner
messages: +
2010-09-29 22:23:39
ncoghlan
set
messages: +
2010-09-29 22:09:30
pitrou
set
messages: +
2010-09-29 21:55:57
ncoghlan
set
messages: +
2010-09-29 20:56:14
ncoghlan
set
messages: +
2010-09-29 20:49:11
ncoghlan
set
messages: +
2010-09-29 09:57:39
pitrou
set
nosy: + pitrou
messages: +
2010-09-28 21:56:31
ncoghlan
set
messages: +
2010-09-28 14:00:57
ncoghlan
set
files: + issue9873_initial_attempt.diff
keywords: + patch
messages: +
2010-09-20 11:42:32
ncoghlan
set
messages: +
2010-09-17 13:43:12
ncoghlan
set
messages: +
2010-09-16 20:10:54
eric.araujo
set
nosy: + eric.araujo
2010-09-16 14:08:18
eric.smith
set
nosy: + eric.smith
2010-09-16 14:01:58
r.david.murray
set
nosy: + r.david.murray
2010-09-16 13:25:10
ncoghlan
create