[issue17289] readline.set_completer_delims() doesn't play well with others

[Roundup Robot]

Roundup Robot added the comment:

New changeset df0afd3ebb70 by Antoine Pitrou in branch '3.3':
Issue #17289: The readline module now plays nicer with external modules or 
applications changing the rl_completer_word_break_characters global variable.
http://hg.python.org/cpython/rev/df0afd3ebb70

New changeset 0f65426009e2 by Antoine Pitrou in branch 'default':
Issue #17289: The readline module now plays nicer with external modules or 
applications changing the rl_completer_word_break_characters global variable.
http://hg.python.org/cpython/rev/0f65426009e2

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Antoine Pitrou]

Antoine Pitrou added the comment:

Thanks for the patch! I made a variable name a bit shorter and also added some 
error checking on the strdup() result.

--
resolution:  -> fixed
stage: patch review -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Roundup Robot]

Roundup Robot added the comment:

New changeset 55c7295aca6c by Antoine Pitrou in branch '2.7':
Issue #17289: The readline module now plays nicer with external modules or 
applications changing the rl_completer_word_break_characters global variable.
http://hg.python.org/cpython/rev/55c7295aca6c

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Bradley Froehle]

Bradley Froehle added the comment:

Patch attached. I implemented this by adding a 'static char *' which
holds the memory we allocate. I did not use the PyState machinery.

--
keywords: +patch
Added file: http://bugs.python.org/file30143/readline_completer_state.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Antoine Pitrou]

Antoine Pitrou added the comment:

Thanks for reporting this. Do you want to contribute a proper patch? You'll 
find instructions at http://docs.python.org/devguide/

--
nosy: +pitrou
stage:  -> needs patch
versions:  -Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Christoph Gohlke]

Changes by Christoph Gohlke :


--
nosy: +cgohlke

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Thomas Kluyver]

Changes by Thomas Kluyver :


--
nosy: +takluyver

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17289] readline.set_completer_delims() doesn't play well with others

[Bradley Froehle]

New submission from Bradley Froehle:

The `readline.set_completer_delims` doesn't play well with others because
it assumes that only it ever allocates or modifies the
rl_completer_word_break_characters buffer.  If other programs modify this
value, for example changing it from heap allocated space to stack
allocated space, the results can be catastrophic.

To remind you, the function essentially works as:

set_completer_delims(PyObject *self, PyObject *args)
{
// ...
free((void*) rl_completer_word_break_characters;
rl_completer_word_break_characters = strdup(break_chars);
// ...
}

where `break_chars` is the user provided string.

Take, for example, R as another programs which changes the readline
completer strings.  When an embedded R instance is initialized (say, using
`r2py`) something similar to the following takes place::

static void
set_rl_completer_word_break_characters(const char *new)
{
static char[201] buffer;
strncpy(buffer, new, 200);
rl_completer_word_break_characters = buffer;
}

static void
initialize_embedded_R(...)
{
// ...
set_rl_completer_word_break_characters(...);
}

As you might expect the next trip through `readline.set_completer_delims`
after initializing R will be catastrophic when we attempt to free a stack
allocate buffer.

I think we should consider modifying the `readline.set_completer_delims`
to store the allocated buffers in the module state::

set_completer_delims(PyObject *self, PyObject *args)
{
// ...
free(_readlinestate_global->break_chars);
rl_completer_word_break_characters = strdup(break_chars);
_readlinestate_global->break_chars = rl_completer_word_break_characters;
// ...
}

This would prevent the segfault and memory leaks, and would render weird
hacks (like https://bitbucket.org/lgautier/rpy2/commits/408bae913653 in
the r2py code) unnecessary.

--
components: Extension Modules
messages: 182882
nosy: bfroehle
priority: normal
severity: normal
status: open
title: readline.set_completer_delims() doesn't play well with others
type: crash
versions: Python 2.7, Python 3.2, Python 3.3, Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com