compile() cannot decode Latin-1 source encodings

[brettcannon]

BPO	3574
Nosy	@loewis, @warsaw, @brettcannon, @vstinner, @benjaminp
Dependencies	bpo-2384: [Py3k] line number is wrong after encoding declaration bpo-3594: PyTokenizer_FindEncoding() never succeeds
Files	fix_latin.diff test_pep3120.py iso.py: Script encoding in ISO-8859-1 and has the #coding header tokenizer_iso-8859-1-patch2.patch tokenizer_iso-8859-1-patch3.patch alt_latin_1.diff: Tweak to Victor's 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 = 'https://github.com/brettcannon'
closed_at = <Date 2008-10-17.03:39:11.888>
created_at = <Date 2008-08-17.00:39:39.566>
labels = ['interpreter-core', 'type-bug', 'release-blocker']
title = 'compile() cannot decode Latin-1 source encodings'
updated_at = <Date 2008-10-17.03:39:11.887>
user = 'https://github.com/brettcannon'

bugs.python.org fields:

activity = <Date 2008-10-17.03:39:11.887>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2008-10-17.03:39:11.888>
closer = 'brett.cannon'
components = ['Interpreter Core']
creation = <Date 2008-08-17.00:39:39.566>
creator = 'brett.cannon'
dependencies = ['2384', '3594']
files = ['11398', '11399', '11697', '11700', '11716', '11722']
hgrepos = []
issue_num = 3574
keywords = ['patch']
message_count = 31.0
messages = ['71251', '71252', '71257', '71258', '71401', '71402', '71403', '71846', '71850', '71852', '71855', '72624', '74287', '74288', '74291', '74294', '74295', '74296', '74299', '74300', '74393', '74415', '74428', '74620', '74642', '74793', '74807', '74811', '74892', '74894', '74895']
nosy_count = 5.0
nosy_names = ['loewis', 'barry', 'brett.cannon', 'vstinner', 'benjamin.peterson']
pr_nums = []
priority = 'release blocker'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue3574'
versions = ['Python 3.0']

[brettcannon]

The following leads to a SyntaxError in 3.0:

  compile(b'# coding: latin-1\nu = "\xC7"\n', '<dummy>', 'exec')

That is not the case in Python 2.6.

[brettcannon]

Looks like Parser/tokenizer.c:check_coding_spec() considered Latin-1 a
raw encoding just like UTF-8. Patch is in the works.

[brettcannon]

Here is a potential fix. It broke test_imp because it assumed that
Latin-1 source files would be encoded at Latin-1 instead of UTF-8 when
returned by imp.new_module(). Doesn't seem like a critical change as the
file is still properly decoded.

[brettcannon]

Attached is a test for test_pep3120 (since that is what most likely
introduced the breakage). It's a separate patch since the source file is
marked as binary and thus can't be diffed by svn diff.

[brettcannon]

Can someone double-check this patch for me? I don't have much experience
with the parser so I want to make sure I am not doing anything wrong.

[brettcannon]

There is a potential dependency on bpo-3594 as it would change how
imp.find_module() acts and thus make test_imp no longer fail in the way
it has.

[benjaminp]

That line dates back to the PEP-263 implementation. Martin?

[loewis]

Since this is marked "release blocker", I'll provide a shallow comment:

I don't think it should be a release blocker. It's a bug in the compile
function, and there are various work-arounds (such as saving the bytes
to a temporary file and executing that one, or decoding the byte string
to a Unicode string, and then compiling the Unicode string). It is
sufficient to fix it in 3.0.1.

I don't think the patch is right: as the test had to be changed, it
means that somewhere, the detection of the encoding declaration now
fails. This is clearly a new bug, but I don't have the time to analyse
the cause further.

In principle, there is nothing wrong with the tokenizer treating latin-1
as "raw" - that only means we don't go through a codec.

[brettcannon]

Actually, the tests don't have to change; if bpo-3594 gets applied
then that change cascades into this issue and negates the need to change
the tests themselves.

As for treating Latin-1 as a raw encoding, how can that be theoretically
okay if the parser assumes UTF-8 and Latin-1 is not a superset of Latin-1?

[loewis]

As for treating Latin-1 as a raw encoding, how can that be theoretically
okay if the parser assumes UTF-8 and Latin-1 is not a superset of Latin-1?

The parser doesn't assume UTF-8, but "ascii+", i.e. it passes all
non-ASCII bytes on to the AST, which then needs to deal with them;
it then could (but apparently doesn't) take into account whether the
internal representation was UTF-8 or Latin-1: see ast.c:decode_unicode
for some remains of that.

The other case (besides string literals) where bytes > 127 matter is
tokenizer.c:verify_identifier; this indeed assumes UTF-8 only (but
could be easily extended to support Latin-1 as well).

The third case where non-ASCII bytes are allowed is comments; there
they are entirely ignored (i.e. it is not even verified that the
comment is well-formed UTF-8).

Removal of the special case should simplify the code; I would agree
that any speedup gained by not going through a codec is irrelevant.
I'm still puzzled why test_imp if the special case is removed.

[brettcannon]

The test_imp stuff has to do with PyTokenizer_FindEncoding().
imp.find_module() only opens the file, passes the file descriptor to
PyTokenizer_FindEncoding() and then returns a file object with the found
encoding.

Problem is that (as bpo-3594 points out), PyTokenizer_FindEncoding()
always fails. That means it assumes only the raw encodings are okay.
With Latin-1 being one of them, it returns the file opened as Latin-1 as
is correct. Removing that case here means PyTokenizer_FindEncoding()
fails, and thus assumes only UTF-8 as a legitimate encoding and opens
the files with the UTF-8 encoding. It took a while to find these two
bugs obviously. =)

[brettcannon]

I have attached a new version of the patch with the changes to test_imp
removed as bpo-3594 fixed the need for the change. I have also
directly uploaded test_pep3120.py since it is flagged as binary and thus
cannot be diffed by svn.