Using percent signs with SafeConfigParser

[Márcio Faustino]

Hi,

Does the SafeConfigParser class correctly detects lone percent signs?
For example, shouldn't the string "100%%" be accepted as a valid
value? Executing the code below should only print one error, instead
it prints 2. (I've tested this with version 2.6.1 on Windows XP.)

It seems the "_badpercent_re" regular expression (an attribute of
SafeConfigParser, with value "%[^%]|%$") is flawed:
- The first alternative "%[^%]" fails with the string "%%_", because
it matches "%_".
- The second alternative "%$" fails with the string "%%", because it
matches "%".

The correct regular expression is "(?<!%)%(?!%)", which passes the
previous tests.

Thanks,

--

from ConfigParser import *
import re

config = SafeConfigParser()

# Expected:
try:
config.set('DEFAULT', 'test', '100%')
except ValueError as error:
print 'Error:', error

# Unexpected:
try:
config.set('DEFAULT', 'test', '100%%')
except ValueError as error:
print 'Error:', error

try:
config._badpercent_re = re.compile('(?<!%)%(?!%)')
config.set('DEFAULT', 'test', '100%%')
except ValueError as error:
print 'Error:', error

 

[Peter Otten]

Does the SafeConfigParser class correctly detects lone percent signs?
For example, shouldn't the string "100%%" be accepted as a valid
value? Executing the code below should only print one error, instead
it prints 2. (I've tested this with version 2.6.1 on Windows XP.)

It seems the "_badpercent_re" regular expression (an attribute of
SafeConfigParser, with value "%[^%]|%$") is flawed:
- The first alternative "%[^%]" fails with the string "%%_", because
it matches "%_".
- The second alternative "%$" fails with the string "%%", because it
matches "%".

I think you are right. Please file a bug report .

The correct regular expression is "(?<!%)%(?!%)", which passes the
previous tests.

IMO this doesn't fix the problem because

(1) it allows "%%%" which is also incorrect
(2) _interpvar_re has already butchered values like "%%(alpha)s" to "%"

Peter

 

[Márcio Faustino]

I think you are right. Please file a bug report .

I will.

IMO this doesn't fix the problem because

(1) it allows "%%%" which is also incorrect

You're right, how about this one "(?<!%)%(?:%%)*(?!%)"?

(2) _interpvar_re has already butchered values like "%%(alpha)s" to "%"

Isn't that expected? I mean, one wouldn't write "%%(alpha)s", but
instead "%%%(alpha)s" or "%(alpha)s" right?

 

[MRAB]

>
> I will.
>
>
> You're right, how about this one "(?<!%)%(?:%%)*(?!%)"?
>

Instead of checking for a 'bad' value, why not check for a 'good' one?

Change:

_badpercent_re = re.compile(r"%[^%]|%$")

to:

_goodpercent_re = re.compile(r"(?:%%|[^%])*$")

and then:

m = self._badpercent_re.search(tmp_value)
if m:

to:

m = self._goodpercent_re.match(tmp_value)
if not m:

 

[Peter Otten]

Isn't that expected? I mean, one wouldn't write "%%(alpha)s", but
instead "%%%(alpha)s" or "%(alpha)s" right?

"%%(alpha)s" would mean that you want the value to be "%(alpha)s" (unlikely,
but possible).

Peter

 

[Peter Otten]

I will.


You're right, how about this one "(?<!%)%(?:%%)*(?!%)"?

Instead of checking for a 'bad' value, why not check for a 'good' one?

Change:

_badpercent_re = re.compile(r"%[^%]|%$")

to:

_goodpercent_re = re.compile(r"(?:%%|[^%])*$")

and then:

m = self._badpercent_re.search(tmp_value)
if m:

to:

m = self._goodpercent_re.match(tmp_value)
if not m:

Configparser also knows a construct "%(whatever)s" which works like Python's
string formatting and takes "whatever" from the "DEFAULT" section of the
config file.

I like your approach, but I think the regex should be

_goodpercent_re = re.compile(r"^(?:%%|%\([^)]+\)s|[^%])*$"

or similar -- what is currently allowed within the parens seems to be an
implementation accident...

Peter