Retrievecontentpack might fail

fsrechia commented 7 years ago

Summary

Extraction of subtitles from content packs might fail depending on zip file folder structure. If the folder subtitles/ itself is listed as a file inside the zip file, loading the content pack fails with

IOError: [Errno 21] Is a directory: '/home/positivo/.kalite/content/srt/pt_BR/subtitles/'

System information

Server Operating system: Linux Open Mandriva 2014 with kernel 4.9.26
Client Operating System: Opensuse Leap 42.2 with kernel 4.4.72-18.12-default
KA Lite Version: 0.17.1
Browser: Google Chrome 59.0.3071.109 64 bits

Traceback

The following error happens when trying to load a custom content pack into kalite

$ kalite-env/bin/kalite manage retrievecontentpack local pt-BR pt-BR.zip
[INFO] [2017-06-23 18:58:47,750] kalite: Installing a local content pack.
[INFO] [2017-06-23 18:58:47,909] kalite: Moving content files to the right place.
[WARNING] [2017-06-23 18:58:48,103] kalite: No pt-BR_icu.js file found in locale_path /home/positivo/.kalite/locale
[WARNING] [2017-06-23 18:58:48,103] kalite: No pt-BR_icu.js file found in locale_path /home/positivo/.kalite/locale
[INFO] [2017-06-23 18:58:48,103] kalite: Writing i18nized js file to /home/positivo/.kalite/content/locale/js/i18n/pt-BR.js
Traceback (most recent call last):
  File "/home/positivo/kalite-env/lib/python2.7/site-packages/kalite/packages/bundled/django/core/management/base.py", line 224, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/positivo/kalite-env/lib/python2.7/site-packages/kalite/packages/bundled/fle_utils/django_utils/command.py", line 141, in execute
    super(LocaleAwareCommand, self).execute(*args, **kwargs)
  File "/home/positivo/kalite-env/lib/python2.7/site-packages/kalite/packages/bundled/django/core/management/base.py", line 263, in execute
    output = self.handle(*args, **options)
  File "/home/positivo/kalite-env/lib/python2.7/site-packages/kalite/distributed/management/commands/retrievecontentpack.py", line 93, in handle
    self.local(*args, **options)
  File "/home/positivo/kalite-env/lib/python2.7/site-packages/kalite/distributed/management/commands/retrievecontentpack.py", line 159, in local
    self.process_content_pack(zf, lang)
  File "/home/positivo/kalite-env/lib/python2.7/site-packages/kalite/distributed/management/commands/retrievecontentpack.py", line 167, in process_content_pack
    extract_subtitles(zf, lang)
  File "/home/positivo/kalite-env/lib/python2.7/site-packages/kalite/distributed/management/commands/retrievecontentpack.py", line 220, in extract_subtitles
    with open(subtitle_dest_path, "w") as dest_fileobj:
IOError: [Errno 21] Is a directory: '/home/positivo/.kalite/content/srt/pt_BR/subtitles/'

How to reproduce

download original pt-BR content pack http://pantry.learningequality.org/downloads/ka-lite/0.17/content/contentpacks/pt-BR.zip
unzip it
update .mo and metadata files as desired
zip it back as pt-BR.zip... the following zip structure is expected:

backend.mo
content.db
frontend.mo
metadata.json
subtitles/
  |
  +-> subtitle1.vtt
  +-> subtitle2.vtt
  +-> subtitle3.vtt
  ...

how to fix it:

Depending on the program used to create the content pack, the subtitles folder is listed as a file itself.

Therefore, retrievecontentpack.py line 220 fails, as it resolves the destination filename to a directory (because subtitle_filename is empty).

I suggest inserting this snippet right after retrievecontentpack.py line 215 to ignore empty filenames:

        if len(subtitle_filename) < 1:
            continue

This is what I did in my local kalite installation and it worked -- I managed to install my custom content pack with additional translations (still not released as a content pack).

Real-life consequences

This issue makes things harder for users trying to update their own content packs, as not all zip tools will omit directories as a file...

benjaoming commented 7 years ago

@fsrechia - thanks for noticing. Would you be able to provide a PR that fixes the issue for you? Supporting customized content packs isn't a big priority for us, but a fix is definitely welcomed and will be merged :)

fsrechia commented 7 years ago

@benjaoming alright, I'll try to do that. thanks!

benjaoming commented 6 years ago

Tested this with a content pack that had both a "subtitles/" folder and a "subtitles" file in the zip root. It works -- and also still works when there isn't a mysterious "subtitle" file :)

learningequality / ka-lite