classification
Title: rlcompleter raises Exception on bad input
Type: behavior Stage:
Components: Library (Lib) Versions: Python 2.5
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: nascheme Nosy List: donlorenzo, georg.brandl, jafo, nascheme
Priority: normal Keywords: easy, patch

Created on 2008-03-07 10:49 by donlorenzo, last changed 2008-05-11 21:04 by georg.brandl. This issue is now closed.

Files
File name Uploaded Description Edit
rlcompleter.patch donlorenzo, 2008-03-07 10:49 catches Name- and AttributeError and instead returns an empty list
rlcompleter_Exception.patch donlorenzo, 2008-04-04 18:06 catches all Exceptions and returns an empty list instead
rlcompleter.rst.diff donlorenzo, 2008-04-18 14:47 note that exceptions will be caught.
Messages (9)
msg63349 - (view) Author: Lorenz Quack (donlorenzo) Date: 2008-03-07 10:49
in line 130 rlcompleter calls eval on the first part (before the last
dot) of the input text.
if that part is not valid it will raise an exception which is likely to
crash a calling application. 

example 1:
==========
import rlcompleter
rlcompleter.Completer().complete("foo.", 0) -> raises NameError

example 2:
==========
import rlcompleter
foo = 5
rlcompleter.Completer().complete("foo.bar.", 0) -> raises AttributeError

the patch puts the eval call in a try-except block and catches Name- and
AttributeErrors and returns an empty list (the behavior when no matches
are found).
msg64206 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2008-03-20 20:49
Is a straightforward patch, but I'd like NAS to comment on the change in
behavior.  Probably would also need a documentation change, are you up
for doing that Lorenz?
msg64414 - (view) Author: Lorenz Quack (donlorenzo) Date: 2008-03-24 13:55
I have no idea what or who NAS is but comments are always welcome :)

concerning the documentation:
I´ve never done it before but there is a first time for everything,
right? I´m currently on vacation so I won´t look into it for at least
another 2 weeks.
msg64453 - (view) Author: Neil Schemenauer (nascheme) * (Python committer) Date: 2008-03-25 02:51
On Thu, Mar 20, 2008 at 08:49:32PM +0000, Sean Reifschneider wrote:
> Is a straightforward patch, but I'd like NAS to comment on the change in
> behavior.  Probably would also need a documentation change, are you up
> for doing that Lorenz?

I doubt that people are relying on complete() to raise NameError or
Attribute error.  I think it would be okay just to trap them and not
change the docs.  A note in NEWS should be good enough.

  Neil
msg64554 - (view) Author: Lorenz Quack (donlorenzo) Date: 2008-03-26 13:35
I was thinking that the code in question could maybe also raise other
exceptions. too bad I´m on vacation and can´t try this out myself.
I believe the regular expression also matches something like 

import rlcompleter
rlcompleter.Completer().complete("1foo.2bar3.smth", 0)

which I guess would result in a SyntaxError. 
would be nice if someone could verify that.

If I´m right I see two possibilities. either change the regular
expression to match only valid python identifieres or also catch
SyntaxErrors.

could there be any other exception? ("In the face of ambiguity, refuse
the temptation to guess.")
msg64936 - (view) Author: Lorenz Quack (donlorenzo) Date: 2008-04-04 18:06
I confirmed that the rlcompleter can raise a SyntaxError on bad input.
Upon further investigation I found that a ReferenceError could also be
raised. I didn't check on other Exceptions.

I attached a new version of the patch where I catch all Errors derived
from Exceptions (not SystemError and KeyboardInterrupt).

while this seems rather drastic I really feel that this step should in
no case raise an Exception. I can't possibly think of a situation where
someone would want that.

Concerning the docs:
"...it will try to evaluate anything without obvious side-effects..."
I would consider raising an exception as a major side-effect so this
patch rather brings the code into accordance with the docs than it
contradicts them.
msg65615 - (view) Author: Lorenz Quack (donlorenzo) Date: 2008-04-18 14:45
I attached a patch for the docs.
It now states that the rlcompleter.Completer will catch and silence all
exceptions raised by evaluating the expression passed to complete()
msg65616 - (view) Author: Lorenz Quack (donlorenzo) Date: 2008-04-18 14:47
In the last patch I used the wrong ticks. I hope it's fine like this.
msg66669 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-05-11 21:04
Committed code and doc patches as r63094. Thanks!
History
Date User Action Args
2008-05-11 21:04:17georg.brandlsetstatus: open -> closed
nosy: + georg.brandl
resolution: accepted
messages: + msg66669
2008-04-18 14:47:52donlorenzosetfiles: - rlcompleter.rst.diff
2008-04-18 14:47:43donlorenzosetfiles: + rlcompleter.rst.diff
messages: + msg65616
2008-04-18 14:45:23donlorenzosetfiles: + rlcompleter.rst.diff
messages: + msg65615
2008-04-04 18:06:21donlorenzosetfiles: + rlcompleter_Exception.patch
messages: + msg64936
2008-03-26 13:35:05donlorenzosetmessages: + msg64554
2008-03-25 02:51:33naschemesetmessages: + msg64453
2008-03-24 13:55:59donlorenzosetmessages: + msg64414
2008-03-20 20:49:31jafosetpriority: normal
assignee: nascheme
messages: + msg64206
keywords: + easy
nosy: + nascheme, jafo
2008-03-07 10:49:56donlorenzocreate