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

gettext doesn't check MO versions #62416

Closed
jwilk mannequin opened this issue Jun 14, 2013 · 14 comments
Closed

gettext doesn't check MO versions #62416

jwilk mannequin opened this issue Jun 14, 2013 · 14 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jwilk
Copy link
Mannequin

jwilk mannequin commented Jun 14, 2013

BPO 18216
Nosy @loewis, @pitrou, @jwilk, @berkerpeksag
Files
  • messages.mo
  • gettext-mo-version.patch
  • gettext-mo-fixup.patch
  • 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 2014-10-28.19:22:39.743>
    created_at = <Date 2013-06-14.20:34:41.407>
    labels = ['type-bug', 'library']
    title = "gettext doesn't check MO versions"
    updated_at = <Date 2014-10-28.19:24:29.569>
    user = 'https://github.com/jwilk'

    bugs.python.org fields:

    activity = <Date 2014-10-28.19:24:29.569>
    actor = 'Aaron1011'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-10-28.19:22:39.743>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2013-06-14.20:34:41.407>
    creator = 'jwilk'
    dependencies = []
    files = ['30587', '36828', '36958']
    hgrepos = []
    issue_num = 18216
    keywords = ['patch']
    message_count = 14.0
    messages = ['191151', '228418', '228662', '228761', '228768', '229571', '229591', '229628', '229629', '230000', '230167', '230168', '230169', '230171']
    nosy_count = 7.0
    nosy_names = ['loewis', 'pitrou', 'jwilk', 'BreamoreBoy', 'python-dev', 'berker.peksag', 'Aaron1011']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18216'
    versions = ['Python 3.5']

    @jwilk
    Copy link
    Mannequin Author

    jwilk mannequin commented Jun 14, 2013

    The MO file format specification[0] reads:
    "A program seeing an unexpected major revision number should stop reading the MO file entirely"
    But Python doesn't pay attention to versions at all.

    As a test-case I attached a MO file with a bogus major revision number. msgunfmt correcly rejects such a file:

    $ msgunfmt messages.mo 
    msgunfmt: file "messages.mo" is not in GNU .mo format

    Yet Python opens it happily:

    >>> import gettext
    >>> t = gettext.GNUTranslations(open("messages.mo", "rb"))
    >>> t.gettext("foo")
    'bar'

    [0] http://www.gnu.org/software/gettext/manual/html_node/MO-Files.html

    @jwilk jwilk mannequin added the stdlib Python modules in the Lib dir label Jun 14, 2013
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Oct 4, 2014

    @jakub does this apply to all Python versions and OSes?

    @jwilk
    Copy link
    Mannequin Author

    jwilk mannequin commented Oct 6, 2014

    I believe so, yes.

    @berkerpeksag berkerpeksag added the type-bug An unexpected behavior, bug, or error label Oct 7, 2014
    @jwilk
    Copy link
    Mannequin Author

    jwilk mannequin commented Oct 7, 2014

    The patch hardcodes 5 as version number in the exception message.

    The specifiction also says that "an unexpected minor revision number means that the file can be read but will not reveal its full contents, when parsed by a program that supports only smaller minor revision numbers". So I think a reasonable thing to do is to accept MO files with an expected major revision but an unexpected minor revision.

    @Aaron1011
    Copy link
    Mannequin

    Aaron1011 mannequin commented Oct 7, 2014

    That sounds good. Should a warning be thrown for an unexpected minor revision?

    @Aaron1011
    Copy link
    Mannequin

    Aaron1011 mannequin commented Oct 17, 2014

    Does anyone have any thoughts about throwing a warning for an unexpected minor revision?

    @pitrou
    Copy link
    Member

    pitrou commented Oct 17, 2014

    The linked docs say: """an unexpected minor revision number means that the file can be read but will not reveal its full contents, when parsed by a program that supports only smaller minor revision numbers""".

    Unless there an important piece of contents that can be missed, I would say a warning is more of a distraction here.

    @Aaron1011
    Copy link
    Mannequin

    Aaron1011 mannequin commented Oct 18, 2014

    Okay, then. I'll just leave it out.

    @Aaron1011
    Copy link
    Mannequin

    Aaron1011 mannequin commented Oct 18, 2014

    I've added a second patch, which properly distinguishes between major and minor revisions, and updates the docs to account for the new behavior.

    @Aaron1011
    Copy link
    Mannequin

    Aaron1011 mannequin commented Oct 25, 2014

    Is there anything that needs to be changed?

    @pitrou
    Copy link
    Member

    pitrou commented Oct 28, 2014

    Aaron, the patch looks fine, I'm going to commit it.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 28, 2014

    New changeset 3b26a0983a3c by Antoine Pitrou in branch 'default':
    Issue bpo-18216: gettext now raises an error when a .mo file has an unsupported major version number. Patch by Aaron Hill.
    https://hg.python.org/cpython/rev/3b26a0983a3c

    @pitrou
    Copy link
    Member

    pitrou commented Oct 28, 2014

    I've pushed it to 3.5 only. I don't think it's worth risking compatibility breakage in bugfix branches. Thank you for your contribution!

    @pitrou pitrou closed this as completed Oct 28, 2014
    @Aaron1011
    Copy link
    Mannequin

    Aaron1011 mannequin commented Oct 28, 2014

    Awesome! Thanks!

    @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
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants