Repository navigation
List index out of range error in PoFileParser._add_message #1134
Description
The below code triggers a list index out of range error when provided a malformed input. This occurs in _add_message at line 235 in babel/messages/pofile.py.
import sys
import io
import babel.messages.pofile
import babel
def main():
input = io.StringIO(open(sys.argv[1], "r").read())
babel.messages.pofile.read_po(input)
if __name__ == "__main__":
main()
Version
babel version 2.16.0
POC File
https://github.com/FuturesLab/POC/blob/main/babel/poc-01
How to trigger:
python filename.py poc-01
Trace report
Traceback (most recent call last):
File "rep.py", line 11, in <module>
main()
File "rep.py", line 7, in main
babel.messages.pofile.read_po(input)
File "/home/gabe/extras/atheris_venv/lib/python3.8/site-packages/babel/messages/pofile.py", line 434, in read_po
parser.parse(fileobj)
File "/home/gabe/extras/atheris_venv/lib/python3.8/site-packages/babel/messages/pofile.py", line 361, in parse
self._finish_current_message()
File "/home/gabe/extras/atheris_venv/lib/python3.8/site-packages/babel/messages/pofile.py", line 250, in _finish_current_message
self._add_message()
File "/home/gabe/extras/atheris_venv/lib/python3.8/site-packages/babel/messages/pofile.py", line 235, in _add_message
string = self.translations[0][1].denormalize()
IndexError: list index out of range
Activity
tomasr8 commentedon Sep 24, 2024
Here's a smaller reproducer:
Neither is a valid PO file so we should ideally raise a
PoFileErrorin this case. @gabe-sherman would you like to open a PR?gabe-sherman commentedon Sep 24, 2024
Hey Tomas! Thanks for the response. I don't have a deep understanding of the way the PO files should be processed so a full PR may be challenging, but if I'm able to get some time I'll certainly check it out :)
tomasr8 commentedon Sep 24, 2024
I think checking if
self.translationsis empty inside_finish_current_message()and raising aPoFileErrorif that's the case might work. In any case, feel free to ask if you need help :)gabe-sherman commentedon Sep 24, 2024
Sounds good, thanks! Should the process exit when this is detected or should we only raise the warning and leave the exception raising up to the value of self.abort_invalid.
tomasr8 commentedon Sep 25, 2024
Right, there's this
abort_invalidflag. I guess if it's set toTrue, we should just raise, and if not maybe we could insert a dummy translation, something like(0, '')(and emit a warning)?gabe-sherman commentedon Sep 25, 2024
Is the invariant here that if the messages list has an element in it, the translations list should also be populated? Adding in this check at the beginning of
_finish_current_messagetriggers this new warning on valid files in cases where bothself.translationsandself.messagesare empty. However, when this check to the size ofself.translationsis moved inside theif self.messages:block it behaves as expected, although I haven't done robust testing yet. I just want to make sure we're not triggering this exception in cases where it's okay that the translations list is empty.tomasr8 commentedon Sep 25, 2024
To be honest, I'm not that familiar with the parser code either, but I think you are right. We should only raise/warn when we call
_finish_current_messagewhileself.messagesis not empty andself.translationsis, i.e. something like this:gabe-sherman commentedon Sep 25, 2024
Yea that's what I was thinking as well :). I'll do some testing on it.
tomasr8 commentedon Oct 27, 2024
I think this can be closed now :)