Moodle
  1. Moodle
  2. MDL-33710

moodle created zip files are not utf-8 compatible

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.4, 2.1, 2.2, 2.3
    • Fix Version/s: 2.4
    • Component/s: General
    • Labels:
    • Testing Instructions:
      Hide

      1/ run phpunit tests
      2/ try extracting the attached file
      3/ try downloading of all assignment from old advanced upload assignment module - make sure some students have names with utf-8 chars in them
      4/ try zipping in file manager (I hope we have this functionality somewhere)
      5/ finally run course backup/restore - this is probably the most important use of zipping/unzipping in moodle

      Show
      1/ run phpunit tests 2/ try extracting the attached file 3/ try downloading of all assignment from old advanced upload assignment module - make sure some students have names with utf-8 chars in them 4/ try zipping in file manager (I hope we have this functionality somewhere) 5/ finally run course backup/restore - this is probably the most important use of zipping/unzipping in moodle
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w28_MDL-33710_m24_utfzip
    • Rank:
      41716

      Description

      Potential solution is to use bit 11 in general file header fields. The problem is that we can not tweak the PHP zip extension itself, luckily the general flag is not checksummed and the zip_archive should allow postprocessing in close() methods.

      This issue is not for extracting of zip files created in other systems.

      the ZIP structure is described at http://www.pkware.com/documents/casestudies/APPNOTE.TXT

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          you can see progress in the linked branch

          Show
          Petr Škoda added a comment - you can see progress in the linked branch
          Hide
          Petr Škoda added a comment -

          the attached test.zip was created by the old assignment module - download all submissions.

          The expected file name is "Admin Žluťáček user ...", feel free to try it in your system.

          So far I tested it in:
          1/ Windows 8 native extract
          2/ 7zip in Windows
          3/ WinZip in Windows
          4/ OSX Lion built-in Archive utility
          5/ The Unarchiver for OSX

          Does not work in:
          1/ info-zip CLI binary (5.x and 6.0) - most probably UTF-8 is not fully implemented yet

          Show
          Petr Škoda added a comment - the attached test.zip was created by the old assignment module - download all submissions. The expected file name is "Admin Žluťáček user ...", feel free to try it in your system. So far I tested it in: 1/ Windows 8 native extract 2/ 7zip in Windows 3/ WinZip in Windows 4/ OSX Lion built-in Archive utility 5/ The Unarchiver for OSX Does not work in: 1/ info-zip CLI binary (5.x and 6.0) - most probably UTF-8 is not fully implemented yet
          Hide
          Petr Škoda added a comment -

          I am working on unit tests now...

          Show
          Petr Škoda added a comment - I am working on unit tests now...
          Hide
          Petr Škoda added a comment -

          I have discovered a serious bug in zip_packer->extract_to_storage(), I am going to file separate issue for backporting of the fix to stable

          I think this would be good to have in 2.3 because not everybody speaks English...

          Show
          Petr Škoda added a comment - I have discovered a serious bug in zip_packer->extract_to_storage(), I am going to file separate issue for backporting of the fix to stable I think this would be good to have in 2.3 because not everybody speaks English...
          Hide
          Petr Škoda added a comment -

          the extra test zip files are there because I will need them for fixing of unzipping, the new unit tests will be handy too

          Show
          Petr Škoda added a comment - the extra test zip files are there because I will need them for fixing of unzipping, the new unit tests will be handy too
          Hide
          Nadav Kavalerchik added a comment -

          It's an upstream (ZIP) issue.
          https://bugs.php.net/bug.php?id=53948&edit=1
          ZIP/UNZIP does not support unicode.

          I wonder if there is any chance we use other compression formats that support Unicode
          Or convert the filenames to English letters (phonetically)

          Show
          Nadav Kavalerchik added a comment - It's an upstream (ZIP) issue. https://bugs.php.net/bug.php?id=53948&edit=1 ZIP/UNZIP does not support unicode. I wonder if there is any chance we use other compression formats that support Unicode Or convert the filenames to English letters (phonetically)
          Hide
          Petr Škoda added a comment -

          Did you read my patches or try it? It modifies the zip archives to actually use proper unicode filenames both when creating and when reading them.

          I am personally not planning to add support for any other archive types, feel free to do it if you want to, technically it should not be very difficult.

          Show
          Petr Škoda added a comment - Did you read my patches or try it? It modifies the zip archives to actually use proper unicode filenames both when creating and when reading them. I am personally not planning to add support for any other archive types, feel free to do it if you want to, technically it should not be very difficult.
          Hide
          Dan Poltawski added a comment -

          Taking integration held issues out of integration (whilst we are keeping master and 23_STABLE in sync).

          Show
          Dan Poltawski added a comment - Taking integration held issues out of integration (whilst we are keeping master and 23_STABLE in sync).
          Hide
          Sam Hemelryk added a comment -

          Thanks Petr, changes looked good and have been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Petr, changes looked good and have been integrated now.
          Hide
          Rossiani Wijaya added a comment -

          This looks great.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This looks great. Test passed.
          Hide
          Dan Poltawski added a comment -

          Congratulations!

          You've made it into the weekly release!

          Thanks for your contribution - here are some random drummers to keep you inspired for the next week!
          http://www.youtube.com/watch?v=_QhpHUmVCmY

          Show
          Dan Poltawski added a comment - Congratulations! You've made it into the weekly release! Thanks for your contribution - here are some random drummers to keep you inspired for the next week! http://www.youtube.com/watch?v=_QhpHUmVCmY
          Show
          Vadim Dvorovenko added a comment - - edited 2.2 & 2.3 backport https://github.com/vadimonus/moodle/moodle.git Branches: MDL-33710 -33753-22 & MDL-33710 -33753-23 Diff: https://github.com/vadimonus/moodle/compare/4db36990ee032eef3b46d06179e85595fade3fbd...MDL-33710-33753-22 https://github.com/vadimonus/moodle/compare/32efb09e62b4a68299c6c9eaafe001cce517e5d0...MDL-33710-33753-23

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: