Opened 6 years ago

Closed 5 years ago

#30405 closed Bug (fixed)

IndexError in _get_lines_from_file when module does not match file contents (via loader)

Reported by: Daniel Hahler Owned by: Hasan Ramezani
Component: Error reporting Version: 2.2
Severity: Normal Keywords:
Cc: Florian Apolloner Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

self = <django.views.debug.ExceptionReporter object at 0x7f2a7908ac18>
filename = '…/project/.venv/lib/python3.7/site-packages/pdb.py'
lineno = 230
context_lines = 7
loader = <_frozen_importlib_external.SourceFileLoader object at 0x7f2a73609278>
module_name = 'pdb'

[23]   …/Vcs/django/django/core/handlers/exception.py(90)response_for_exception()
-> response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
[24]   …/Vcs/django/django/core/handlers/exception.py(125)handle_uncaught_exception()
-> return debug.technical_500_response(request, *exc_info)
[25]   …/Vcs/django/django/views/debug.py(94)technical_500_response()
-> html = reporter.get_traceback_html()
[26]   …/Vcs/django/django/views/debug.py(333)get_traceback_html()
-> c = Context(self.get_traceback_data(), use_l10n=False)
[27]   …/Vcs/django/django/views/debug.py(264)get_traceback_data()
-> frames = self.get_traceback_frames()
[28]   …/Vcs/django/django/views/debug.py(427)get_traceback_frames()
-> filename, lineno, 7, loader, module_name,

 385             try:
 386                 context_line = source[lineno]
 387             except:
 388                 __import__('pdb').set_trace()
 389  ->         post_context = source[lineno + 1:upper_bound]
 390
 391             return lower_bound, pre_context, context_line, post_context
(Pdb++) source
['# this file is needed to hijack pdb without eggs', 'import os.path', "pdb_path = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'pdb.py')", 'with open(pdb_path) as f:', "    exec(compile(f.read(), pdb_path, 'exec'))"]

It uses the loader (https://github.com/django/django/blob/47885278c669dd7a13a4c3ff7e58e1cbe88af385/django/views/debug.py#L351), which picks up the pth, and then the contents does not match the expected line number.

I think it should maybe always use the given filename?!

Change History (8)

comment:1 by Carlton Gibson, 6 years ago

Hey Daniel. Can you give some reproduce steps that are a little easier to follow please? Thanks.

comment:2 by Daniel Hahler, 6 years ago

It happened for me while using pdb, and pdb itself crashed (https://github.com/antocuni/pdb/issues/198).

I am using pdb++/pdbpp, which replaces the original "pdb" module itself (https://github.com/antocuni/pdb/blob/81f8ea9f09d9179aceefe9676e47254a8cd95d05/pdb.py#L140-L154).

So in general the issue might not be very common, but I wanted to report it anyway after investigating, since I think that error handling should be robust by itself.

I guess you could simulate it in a unittest by making loader.get_source(module_name) not match the source.

comment:3 by Florian Apolloner, 6 years ago

Triage Stage: UnreviewedAccepted

I think I have seen similar things in the past when using Jinja, in this case the source would be the HTML file which is generally shorter than the generated py code. If you could supply a Unittest, that would be perfect.

comment:4 by Florian Apolloner, 6 years ago

Cc: Florian Apolloner added

comment:5 by Hasan Ramezani, 5 years ago

Owner: set to Hasan Ramezani
Status: newassigned

@Florian, as far as I understand the solution would be something like this. am I right?

diff --git a/django/views/debug.py b/django/views/debug.py
index 86da47ee20..8af1c2e1ca 100644
--- a/django/views/debug.py
+++ b/django/views/debug.py
@@ -349,7 +349,7 @@ class ExceptionReporter:
         if hasattr(loader, 'get_source'):
             try:
                 source = loader.get_source(module_name)
-            except ImportError:
+            except Exception:
                 pass
             if source is not None:
                 source = source.splitlines()

If you agree with me I can prepare a patch and include the test.

comment:6 by Hasan Ramezani, 5 years ago

Has patch: set
Version 0, edited 5 years ago by Hasan Ramezani (next)

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In e8de188c:

Refs #30405 -- Added ExceptionReporter._get_source().

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 4b78546e:

Fixed #30405 -- Fixed source code mismatch crash in ExceptionReporter.

Note: See TracTickets for help on using tickets.
Back to Top