msg187760 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-04-25 07:32 |
The base64 module is currently restricted specifically to bytes and bytearray objects. By using memoryview, it could effectively decode any input type that provides an 8-bit C contiguous view of the underlying data.
|
msg187778 - (view) |
Author: Kushal Das (kushal.das) *  |
Date: 2013-04-25 11:52 |
Working on this.
|
msg187837 - (view) |
Author: Kushal Das (kushal.das) *  |
Date: 2013-04-26 07:26 |
A patch with tests update to use memoryview in base64 module.
|
msg187860 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-04-26 14:52 |
Not only memoryview should be supported, but any class which supports the buffer protocol and is C-contiguous (i.e. array.array).
|
msg187897 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-04-27 11:28 |
Yes, this should probably be done the other way around: wrap the base64 argument in a memoryview, and encode/decode from it.
|
msg187911 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-04-27 15:27 |
As Serhiy and Antoine noted, what I had in mind here was to try to wrap the input in a memoryview if it wasn't an instance of str, bytes or bytearray.
An existing memoryview will be passed back unmodified, while something like array.array will provide a view into its raw data for encoding or decoding. The tests would then cover both memoryview and array.array to ensure it was all working as expected.
|
msg187919 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-04-27 18:07 |
Note that some functions use bytes/bytearray methods and an argument should converted to bytes for them. Other functions use C functions which supports the buffer protocol and do not required any conversion.
|
msg187984 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-04-28 15:06 |
Here is a patch which allows bytes-like arguments in the base64 module. I just removed type checking if underlying function raises an exception with an appropriate message. I'm not sure about b32encode(), perhaps we can left an exception from memoryview().
|
msg189537 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-05-18 18:16 |
Nick, what you say about the patch?
|
msg189580 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-05-19 11:39 |
Oops, my review comments don't actually make sense because I looked at the patch in isolation, rather than checking the full context in the module. Sorry about that.
We have 2 different cases to deal with, only one of which currently has a helper function.
I suggest renaming _bytes_from_decode_data to "_bytes_for_decoding" and adding "_bytes_for_encoding". The difference between them is the implicit encoding of pure ASCII strings to bytes in the decoding case and the details of the error message thrown.
The encoding and decoding functions should then use the appropriate coercion helper for both the input data and for altchars.
|
msg189793 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-05-22 06:53 |
Thank you Ezio and Nick for your comments.
> I suggest renaming _bytes_from_decode_data to "_bytes_for_decoding" and adding "_bytes_for_encoding".
I rather think a TypeError exception raised by `memoryview(s).tobytes()` is good enough and we don't need a special wrapper.
|
msg189858 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-05-23 10:28 |
We now also need to update the new footnote that I added for issue 17844 in http://hg.python.org/cpython/rev/801567d6302c
|
msg189860 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-05-23 11:33 |
The codec uses the old API that breaks the encoded output up into multiple lines.
The new patch:
1. Also changes the behaviour of the older de/encodebytes API
2. Checks that all the defined binary transform codecs actually support memoryview as input for both encoding and decoding (and that the data roundtrips correctly)
|
msg189861 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-05-23 11:36 |
Just adding a note for easier cross-referencing: the old behaviour of these functions was one of the culprits that led to the removal of the codec aliases as discussed in issue 7475
|
msg189900 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-05-24 07:01 |
It works only if input also supports slicing and this slicing is corresponded to bytes-slicing. It perhaps doesn't catch a non C-continuous buffers.
Actually we need something like (unlikely cast() doesn't work with empty buffers):
s = memoryview(s)
if not s:
return b''
s = s.cast('B')
...
|
msg198832 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-10-02 12:54 |
binascii already only supports simple C contiguous buffers, expanding it and the base64 module to handle anything else should be a separate RFE.
|
msg198835 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-10-02 13:05 |
However, _input_type_check should enforce that (as binascii does), so I'll add that before committing.
|
msg198843 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-10-02 14:48 |
After working through this, I found that the modern base64 API just relies on the checks in binascii. All that checks for is:
1. Can the data by exported using PyBUF_SIMPLE?
2. Is it C contiguous?
It completely ignores the number of dimensions and the format information. I added tests to at least capture this behaviour, even though it seems a little dubious to me.
For the legacy API, I didn't relax the input checks that far - the legacy API will still complain if there is more than 1 dimension and if the format code isn't one of 'c', 'b' or 'B'. That's already substantially more permissive than what it supported in previous versions.
Just running the full test suite now, will push after that finishes.
|
msg198844 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-10-02 15:03 |
New changeset d90f25e1a705 by Nick Coghlan in branch 'default':
Close #17839: support bytes-like objects in base64 module
http://hg.python.org/cpython/rev/d90f25e1a705
|
msg202752 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-11-13 14:25 |
New changeset e53984133740 by Nick Coghlan in branch 'default':
Issue #17839: mention base64 change in What's New
http://hg.python.org/cpython/rev/e53984133740
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:44 | admin | set | github: 62039 |
2013-11-13 14:25:23 | python-dev | set | messages:
+ msg202752 |
2013-10-02 15:03:44 | python-dev | set | status: open -> closed
nosy:
+ python-dev messages:
+ msg198844
resolution: fixed stage: patch review -> resolved |
2013-10-02 14:48:52 | ncoghlan | set | messages:
+ msg198843 |
2013-10-02 13:05:17 | ncoghlan | set | assignee: ncoghlan messages:
+ msg198835 |
2013-10-02 12:54:56 | ncoghlan | set | dependencies:
- Allow memoryview.cast() for empty views messages:
+ msg198832 |
2013-10-01 21:48:18 | serhiy.storchaka | set | dependencies:
+ Allow memoryview.cast() for empty views |
2013-05-24 07:01:59 | serhiy.storchaka | set | messages:
+ msg189900 |
2013-05-23 11:36:55 | ncoghlan | set | messages:
+ msg189861 |
2013-05-23 11:33:40 | ncoghlan | set | files:
+ issue17839_base64_buffer_input_3.patch
messages:
+ msg189860 |
2013-05-23 10:28:34 | ncoghlan | set | messages:
+ msg189858 |
2013-05-22 06:53:48 | serhiy.storchaka | set | files:
+ base64_buffer_input_2.patch
messages:
+ msg189793 |
2013-05-19 11:39:03 | ncoghlan | set | messages:
+ msg189580 |
2013-05-18 18:16:22 | serhiy.storchaka | set | messages:
+ msg189537 |
2013-04-29 14:33:59 | vstinner | set | nosy:
+ vstinner
|
2013-04-28 15:06:05 | serhiy.storchaka | set | files:
+ base64_buffer_input.patch
messages:
+ msg187984 |
2013-04-27 18:07:21 | serhiy.storchaka | set | messages:
+ msg187919 |
2013-04-27 15:27:12 | ncoghlan | set | messages:
+ msg187911 |
2013-04-27 11:28:42 | pitrou | set | nosy:
+ pitrou messages:
+ msg187897
|
2013-04-26 14:52:47 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg187860
|
2013-04-26 11:46:44 | ezio.melotti | set | nosy:
+ ezio.melotti
stage: needs patch -> patch review |
2013-04-26 07:26:50 | kushal.das | set | files:
+ issue17839_v1.patch keywords:
+ patch messages:
+ msg187837
|
2013-04-25 13:55:00 | barry | set | nosy:
+ barry
|
2013-04-25 11:52:37 | kushal.das | set | nosy:
+ kushal.das messages:
+ msg187778
|
2013-04-25 10:51:23 | serhiy.storchaka | set | dependencies:
+ Add base64 module tests for a bytearray argument |
2013-04-25 07:49:12 | ncoghlan | link | issue7475 dependencies |
2013-04-25 07:32:34 | ncoghlan | create | |