Repository navigation
traceback.extract_stack raises exception if source filename is pyc and exists on disk #92336
Description
Bug report
If you compile a python file with the cfile as a pyc and it exists on disk, then if you extract the stack when this module is in the stack, linecache will try to load the file as utf-8 and explode.
Simple proof of concept below:
import py_compile
import sys
import traceback
from tempfile import NamedTemporaryFile
with NamedTemporaryFile(suffix=".py") as tmp_f:
tmp_f.write(b"""\
def call(func):
return func()
""")
tmp_f.flush()
py_compile.compile(tmp_f.name, dfile="/tmp/crash.pyc", cfile="/tmp/crash.pyc")
sys.path.insert(0, "/tmp")
import crash
crash.call(traceback.print_stack)results in a traceback of
Traceback (most recent call last):
File "/home/huw/poc.py", line 16, in
crash.call(traceback.print_stack)
File "/tmp/crash.pyc", line 2, in call
File "/usr/lib/python3.9/traceback.py", line 190, in print_stack
print_list(extract_stack(f, limit=limit), file=file)
File "/usr/lib/python3.9/traceback.py", line 211, in extract_stack
stack = StackSummary.extract(walk_stack(f), limit=limit)
File "/usr/lib/python3.9/traceback.py", line 366, in extract
f.line
File "/usr/lib/python3.9/traceback.py", line 288, in line
self._line = linecache.getline(self.filename, self.lineno).strip()
File "/usr/lib/python3.9/linecache.py", line 30, in getline
lines = getlines(filename, module_globals)
File "/usr/lib/python3.9/linecache.py", line 46, in getlines
return updatecache(filename, module_globals)
File "/usr/lib/python3.9/linecache.py", line 137, in updatecache
lines = fp.readlines()
File "/usr/lib/python3.9/codecs.py", line 322, in decode
(result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd3 in position 9: invalid continuation byte
$ hd /tmp/crash.pyc
00000000 61 0d 0d 0a 00 00 00 00 3e d3 73 62 22 00 00 00 |a.......>.sb"...|
^ position 9
Your environment
macOS 11.6.1 (x64): 3.7.4, 3.8.1, 3.9.1, 3.10.0
Debian Buster (x64): 3.7.3, 3.9.2
Debian Bullseye (x64): 3.9.2
Activity
gpshead commented on May 8, 2022
From the py_compile.compile docs: "If dfile is specified, it is used as the name of the source file in error messages instead of file."
dfile= in your example code is pointing to a .pyc file when it is supposed to point to a .py source file if specified. I'm not sure this is a bug, the generated crash.pyc was constructed to point to a non-.py-source file as its source.
huwcbjones commented on May 8, 2022
The pycompile docs do not say this. From the sentence you quoted (and the rest of the docs) there are no requirements on what the format of dfile must be. It just says "source file".
The docs also clearly state "used for error messages". In no way shape or form would I then expect the error handling path to open the file, I expected the dfile value to be shoved straight into the traceback.
From my PoV, there are two ways to address this:
iritkatriel commented on Jun 28, 2022
A pyc is not a source file, it's a compiled bytecode file.
The source file is used to put source code lines in the traceback. Obviously a pyc won't help with that.
It allows the UnicodeDecodeError to propagate to the caller. Seem pretty clear to me.
I don't think there is anything to fix here.
huwcbjones commented on Jun 28, 2022
Throw an exception if source file has a .pyc suffix?
Update to the docs to clear this up maybe?
iritkatriel commented on Jun 28, 2022
It did throw an exception.
The docs are clear that it should be a source file.
huwcbjones commented on Jun 28, 2022
Sure, but if you'd like to spend an entire day in the bowels of django's template loader figuring out why on earth traceback is exploding in a ball of flames for unbeknownst reasons, then be my guest.
But, for your sanity (and the lack thereof of mine), I would strongly recommend against doing so.
It took a fair amount of printf debugging to figure out that the original exception was raised in traceback due to the aforementioned UnicodeDecodeError which was then being caught by django under the assumption that a perfectly valid template was not utf-8 encoded before blowing the entire stack up.
All because of a single mention of "source file" in the docs that did not highlight how paramount it was not point also have this point at a pyc.
So, I think you can understand why I'm pretty insistent that something should be done rather than "Meh, don't do that. Docs are perfect and the exception message is really helpful".
iritkatriel commented on Jun 28, 2022
I understand that you are upset because you had a hard time debugging this. I'd suggest you think about how unit testing would have made it easier to find this problem (it's not an intermittent issue - a single test covering this function call would have caught it).
Are you saying that you read the docs, misunderstood them, and then wrote your code with this bug in it based on that misunderstanding?
huwcbjones commented on Jun 28, 2022
I'm not upset, just frustrated because I know how easy it was to cause something that took a long time to diagnose. I don't want someone else to do the same and I believe that there is something that can be done to prevent it from happening again.
Unittests would not have helped me, because all 12k of them passed. The compilation runs as part of our build system, so it was only when the database migrations were being run in the appliance chroot that django then exploded whilst loading the template engine. In the chroot/appliance we only have the binary files, however their location on disk is the same as the dfile when they were compiled (important for later). I couldn't reproduce it in my dev environment because of the difference in file system layout.
My major gripe is that in the docs the only information about dfile is
name of the source file was the key bit of information I gleaned from reading that sentence a few times.
But had I known that traceback generation would read the source file (dfile parameter) from disk (if it existed) to get the error location, I wouldn't have opened the issue. The error location information is still present/correct when the source file doesn't exist. However, when the "sourcefile" exists but isn't valid, instead of falling back to the "not found" path, it allows a UnicodeDecodeError to make it's way back up the stack?
iritkatriel commented on Jun 28, 2022
Fair enough, we can state that the source file is read.
iritkatriel commented on Jun 29, 2022
Actually you are right about linecache as well: the doc for getline says "This function will never raise an exception — it will return '' on errors".
So it should swallow the UnicodeDecodeError from fp.readlines().
gh-92336: linecache.getline should not raise exceptions on decoding e…
pythongh-92336: linecache.getline should not raise exceptions on deco…
pythongh-92336: linecache.getline should not raise exceptions on deco…
gh-92336: linecache.getline should not raise exceptions on decoding e…
gh-92336: linecache.getline should not raise exceptions on decoding e…
iritkatriel commented on Jun 30, 2022
Thank you @huwcbjones . The documentation change and the linecache bugfix have been pushed to all versions from 3.10.
pythongh-92336: linecache.getline should not raise exceptions on deco…