Add multibyte encoding support to pyexpat

[serhiy-storchaka]

BPO	18059
Nosy	@malemburg, @doerwalter, @amauryfa, @ncoghlan, @scoder, @vstinner, @serhiy-storchaka
Files	pyexpat_multibyte_encodings_4.patch pyexpat_multibyte_encodings_5.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 = None
created_at = <Date 2013-05-25.18:41:28.221>
labels = ['extension-modules', 'expert-XML', 'type-feature', '3.7']
title = 'Add multibyte encoding support to pyexpat'
updated_at = <Date 2017-03-27.10:33:59.091>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2017-03-27.10:33:59.091>
actor = 'doerwalter'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'XML']
creation = <Date 2013-05-25.18:41:28.221>
creator = 'serhiy.storchaka'
dependencies = []
files = ['30381', '31759']
hgrepos = []
issue_num = 18059
keywords = ['patch']
message_count = 16.0
messages = ['189989', '189995', '190011', '190022', '190024', '190064', '190070', '197686', '197708', '197725', '203839', '203898', '203906', '203919', '290483', '290576']
nosy_count = 7.0
nosy_names = ['lemburg', 'doerwalter', 'amaury.forgeotdarc', 'ncoghlan', 'scoder', 'vstinner', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue18059'
versions = ['Python 3.7']

[serhiy-storchaka]

It is possible to add the support of most multibyte encodings to pyexpat.

There are several ways to do this:

1. Generate maps with a special script and add generated file to repository. After adding or updating a multibyte encoding this file should be regenerated.

2. Generate maps on fly. It requires more time for first use of the encoding, but allows support of arbitrary encoding which compatible with expat.

[amauryfa]

I guess GB18030 can't be supported at all?

[serhiy-storchaka]

Here is a patch which implements first way.

Yes, looks as followed encodings could not be supported at all: euc-kr, gb18030, iso2022-kr, utf-7, cp037, cp424, cp500, cp864, cp875, cp1026, cp1140, utf_32, utf_32_be, utf_32_le.

[amauryfa]

Then you should also remove the "Make it as simple as possible" comment :-/

[serhiy-storchaka]

It is still simple enough.

[serhiy-storchaka]

Patch updated. Fixed an error in the encodings generator and added additional compatibility check for 8-bit encodings in PyUnknownEncodingHandler().

Feel free to bikesheed the encodings generator.

[serhiy-storchaka]

Patch updated. Yet some tests added and yet some bugs fixed.

[scoder]

I don't think I have my head deep enough in the encodings implementation to say that this is the correct/best way to do it, but the patch looks mostly reasonable to me and would be a helpful addition.

I have two comments on the pyexpat_encoding_convert() function:

1. I can't see a safe-guard against reading beyond the data buffer. What if s already points to the last byte and we are trying to read two or three bytes to decode them? I wouldn't be surprised to see that this kind of input can be crafted.

2. Creating a throw-away Unicode object through a named decoder looks like a huge overhead for decoding two bytes. It might be considered an optimisation to change that, but if you are really trying to parse a longer XML document with lots of Japanese text in it (i.e. if you actually *need* this feature), it will most likely end up being way too slow to make any real use of it.

I think that both points should be addressed before this gets added.

[serhiy-storchaka]

1. Expat itself responsible for this guard. It has all necessary information and provides an input of required size for custom converter.

2. Yes, this is a problem. I'm working on another approach, when full encoding table built at first request for the encoding (and than cache it). It makes decoding individual characters fast, but requires about 0.5 sec for initialization. Is such approach more suitable?

[serhiy-storchaka]

Here is a totally rewritten patch, which builds decoding table at first request for encoding and save it in the cache. Decoding should be very fast.

Do you have large testing XML files with multibyte encodings? Could you please measure the time of parsing this files and for comparision the time of parsing this files encoded with utf-8 and utf-16?

[serhiy-storchaka]

If anybody is interested in support of multibyte encodings in XML parser, it is time to make a review.

[vstinner]

I'm not sure that multibyte encodings other than UTF-8 are used in the world. I'm not convinced that we should support them. If the changes are small, it's maybe not a bad thing. Do you know which applications use such codecs?

pyexpat_encoding_create() looks like an heuristic. How many multibyte codecs can be used with your patch? A whitelist of multibyte codecs may be less reliable. What do you think?