classification
Title: gettext doesn't check MO versions
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Aaron1011, BreamoreBoy, berker.peksag, jwilk, loewis, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2013-06-14 20:34 by jwilk, last changed 2014-10-28 19:24 by Aaron1011. This issue is now closed.

Files
File name Uploaded Description Edit
messages.mo jwilk, 2013-06-14 20:34
gettext-mo-version.patch Aaron1011, 2014-10-06 19:01 review
gettext-mo-fixup.patch Aaron1011, 2014-10-18 04:42 review
Messages (14)
msg191151 - (view) Author: Jakub Wilk (jwilk) Date: 2013-06-14 20:34
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
msg228418 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-10-04 00:30
@Jakub does this apply to all Python versions and OSes?
msg228662 - (view) Author: Jakub Wilk (jwilk) Date: 2014-10-06 11:44
I believe so, yes.
msg228761 - (view) Author: Jakub Wilk (jwilk) Date: 2014-10-07 09:57
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.
msg228768 - (view) Author: Aaron Hill (Aaron1011) * Date: 2014-10-07 15:12
That sounds good. Should a warning be thrown for an unexpected minor revision?
msg229571 - (view) Author: Aaron Hill (Aaron1011) * Date: 2014-10-17 09:56
Does anyone have any thoughts about throwing a warning for an unexpected minor revision?
msg229591 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-17 18:05
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.
msg229628 - (view) Author: Aaron Hill (Aaron1011) * Date: 2014-10-18 03:12
Okay, then. I'll just leave it out.
msg229629 - (view) Author: Aaron Hill (Aaron1011) * Date: 2014-10-18 04:42
I've added a second patch, which properly distinguishes between major and minor revisions, and updates the docs to account for the new behavior.
msg230000 - (view) Author: Aaron Hill (Aaron1011) * Date: 2014-10-25 13:51
Is there anything that needs to be changed?
msg230167 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-28 19:14
Aaron, the patch looks fine, I'm going to commit it.
msg230168 - (view) Author: Roundup Robot (python-dev) Date: 2014-10-28 19:20
New changeset 3b26a0983a3c by Antoine Pitrou in branch 'default':
Issue #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
msg230169 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-28 19:22
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!
msg230171 - (view) Author: Aaron Hill (Aaron1011) * Date: 2014-10-28 19:24
Awesome! Thanks!
History
Date User Action Args
2014-10-28 19:24:29Aaron1011setmessages: + msg230171
2014-10-28 19:22:39pitrousetstatus: open -> closed
resolution: fixed
messages: + msg230169

stage: patch review -> resolved
2014-10-28 19:20:44python-devsetnosy: + python-dev
messages: + msg230168
2014-10-28 19:14:58pitrousetmessages: + msg230167
versions: - Python 2.7, Python 3.4
2014-10-25 13:51:01Aaron1011setmessages: + msg230000
2014-10-18 04:42:58Aaron1011setmessages: + msg229629
2014-10-18 04:42:17Aaron1011setfiles: + gettext-mo-fixup.patch
2014-10-18 04:35:30Aaron1011setfiles: - gettext-mo-fix-minor-version.patch
2014-10-18 04:33:55Aaron1011setfiles: + gettext-mo-fix-minor-version.patch
2014-10-18 03:12:44Aaron1011setmessages: + msg229628
2014-10-17 18:05:31pitrousetnosy: + pitrou
messages: + msg229591
2014-10-17 09:56:20Aaron1011setmessages: + msg229571
2014-10-07 15:12:52Aaron1011setmessages: + msg228768
2014-10-07 09:57:25jwilksetmessages: + msg228761
2014-10-07 03:58:43berker.peksagsetnosy: + berker.peksag
stage: patch review
type: behavior

versions: + Python 2.7, Python 3.4, Python 3.5
2014-10-06 19:01:06Aaron1011setfiles: + gettext-mo-version.patch
nosy: + Aaron1011
keywords: + patch
2014-10-06 11:44:08jwilksetmessages: + msg228662
2014-10-04 00:30:00BreamoreBoysetnosy: + loewis, BreamoreBoy
messages: + msg228418
2013-06-14 20:34:41jwilkcreate