Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

locale.format() problems with decimal separator #46774

Closed
mishok13 mannequin opened this issue Mar 31, 2008 · 10 comments
Closed

locale.format() problems with decimal separator #46774

mishok13 mannequin opened this issue Mar 31, 2008 · 10 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@mishok13
Copy link
Mannequin

mishok13 mannequin commented Mar 31, 2008

BPO 2522
Nosy @warsaw, @birkenfeld, @pitrou, @bitdancer
Files
  • locale.diff: path that (partly) fixes incorrect locale.format() behavior with malformed strings
  • issue2522.patch: patch and tests
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2009-04-01.03:51:14.509>
created_at = <Date 2008-03-31.17:08:08.082>
labels = ['type-bug', 'library']
title = 'locale.format() problems with decimal separator'
updated_at = <Date 2010-11-10.00:06:52.326>
user = 'https://bugs.python.org/mishok13'

    bugs.python.org fields:

    activity = <Date 2010-11-10.00:06:52.326>
actor = 'barry'
assignee = 'none'
closed = True
closed_date = <Date 2009-04-01.03:51:14.509>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2008-03-31.17:08:08.082>
creator = 'mishok13'
dependencies = []
files = ['9918', '13458']
hgrepos = []
issue_num = 2522
keywords = ['patch']
message_count = 10.0
messages = ['64787', '64810', '84340', '84343', '84347', '84416', '84555', '84974', '120910', '120911']
nosy_count = 5.0
nosy_names = ['barry', 'georg.brandl', 'pitrou', 'mishok13', 'r.david.murray']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue2522'
versions = ['Python 3.1', 'Python 2.7']
    @mishok13
    Copy link
    Mannequin Author

    mishok13 mannequin commented Mar 31, 2008

    locale.format() doesn't insert correct decimal separator to string
    representation when 'format' argument has '\r' or '\n' symbols in it.
    This bug has been reproduced on Python 2.5.2 and svn-trunk.

    Python 2.4.5 (#2, Mar 12 2008, 14:42:24)
[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import locale
>>> locale.setlocale(locale.LC_ALL, "ru_RU.UTF-8")
'ru_RU.UTF-8'
>>> a = 1.234
>>> print locale.format("%f", a)
1,234000
>>> print locale.format("%f\n", a)
1,234000

>>> print locale.format("%f\r", a)
1,234000


Python 2.6a1+ (trunk:62083, Mar 31 2008, 19:24:56)
[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu6)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import locale
>>> locale.setlocale(locale.LC_ALL, "ru_RU.UTF-8")
'ru_RU.UTF-8'
>>> a = 1.234
>>> print locale.format("%f", a)
1,234000
>>> print locale.format("%f\n", a)
1.234000

>>> print locale.format("%f\r", a)
1.234000
Python 2.5.2 (r252:60911, Mar 12 2008, 13:36:25)
[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import locale
>>> locale.setlocale(locale.LC_ALL, "ru_RU.UTF-8")
'ru_RU.UTF-8'
>>> a = 1.234
>>> print locale.format("%f", a)
1,234000
>>> print locale.format("%f\n", a)
1.234000

>>> print locale.format("%f\r", a)
1.234000

    @mishok13 mishok13 mannequin added the type-bug An unexpected behavior, bug, or error label Mar 31, 2008
    @mishok13
    Copy link
    Mannequin Author

    mishok13 mannequin commented Apr 1, 2008

    I've uploaded a patch that fixes this concrete issue, though
    locale.format() continues to silently ignore other types of malformed
    strings (e.g. locale.format('%fSPAMf')).
    I don't think this is correct behavior. Maybe there should be reg-exp
    that locale.format() will use to avoid such issues.

    @mishok13 mishok13 mannequin added the stdlib Python modules in the Lib dir label Apr 1, 2008
    @bitdancer
    Copy link
    Member

    This bug is more subtle than it first appears. As far as I've been able
    to figure out, there is in fact no way to reliably detect that there is
    non-format text after the format specifier short of completely parsing
    the format specifier. I went through several possibilities and found a
    counter example for each that shows it would introduce new bugs:

    See if last character of formatted string is different from last char of
    formatter: format('%s', 'things') would then incorrectly be an error.

    Make sure last character of format string is a valid format character:
    format('%fx', a) would be valid, but it has a 'x' on the end which is
    not part of the format string. (The suggested patch has a false
    negative in this case as well.)

    Check for a decimal in the formatted string and if it didn't get
    transformed, complain: format('%s', '1.234') would fail incorrectly.

    One could argue that at least \n and \r should be checked for since the
    output from those cases is least obviously "wrong", but I don't think
    that is a strong argument. The extra control character is almost as
    "visible" in the output as the trailing 'x' would be in the above
    example, and the effects of the trailing x are equally mysterious.

    To fix this correctly would require reimplementing format parsing in the
    locale module, which would be a maintenance headache.

    I'm inclined to close this "won't fix", unless someone can come up with
    a heuristic that won't give false positives or false negatives.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 28, 2009

    AFAIK, locale.format() is supposed to be used with a single format
    specifier, not a complete format string. It's up to you to concatenate
    the various parts afterwards.

    @bitdancer
    Copy link
    Member

    That is true, however the code contains the comment "this is only for
    one-percent-specifier strings and this should be checked", implying
    that the intent is to make sure only a single format specifier
    has been passed. I don't think it is reasonable to perfect that
    check, however.

    @bitdancer
    Copy link
    Member

    It occured to me last night that it could be checked using a regular
    expression, and indeed the locale module already has a regular
    expression that matches percent codes. I've uploaded a patch that uses
    this regex to fix this issue. I've removed 2.6 and 3.0 as this change
    could break existing code that is misusing format.

    I added georg.brandl to the nosy list since svn blame shows him as the
    author of the code being modified.

    @mishok13
    Copy link
    Mannequin Author

    mishok13 mannequin commented Mar 30, 2009

    Nice to see this moving forward. Your patch looks nicer than my naive
    approach and I hope it's going to be applied. Thanks for investigation. :)

    @bitdancer
    Copy link
    Member

    Fixed in r70936/r70938.

    @warsaw
    Copy link
    Member

    warsaw commented Nov 10, 2010

    Hmm. See bug 10379 for fallout from this change. I'm not saying it should be reverted but see that issue for further discussion.

    @warsaw
    Copy link
    Member

    warsaw commented Nov 10, 2010

    I mean bpo-10379

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @warsaw @bitdancer @pitrou