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

      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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Petr Skoda added a comment -

            you can see progress in the linked branch

            Show
            Petr Skoda added a comment - you can see progress in the linked branch
            Hide
            Petr Skoda 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 Skoda 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 Skoda added a comment -

            I am working on unit tests now...

            Show
            Petr Skoda added a comment - I am working on unit tests now...
            Hide
            Petr Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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: