Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-43570

Incorrect mimetypes for: bdoc, cdoc, ddoc

    Details

    • Testing Instructions:
      Hide

      Testing the upgrade
      1. Upload one of each file (ddoc, bdoc and cdoc file)
      2. Upgrade
      3. Check that the mimetype has been updated

      Test the improvement
      1. Upload a ddoc, bdoc or cdoc file to a course (ddoc is most common, fake file will work).
      2. Make sure the file is automatically downloaded, it is useless if Moodle tries to open it.

      Show
      Testing the upgrade 1. Upload one of each file (ddoc, bdoc and cdoc file) 2. Upgrade 3. Check that the mimetype has been updated Test the improvement 1. Upload a ddoc, bdoc or cdoc file to a course (ddoc is most common, fake file will work). 2. Make sure the file is automatically downloaded, it is useless if Moodle tries to open it.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull 2.6 Branch:
      MDL-43570-M26
    • Pull Master Branch:

      Description

      bdoc, cdoc, ddoc added to filetype list.

      These filetypes are documents with digital signatures. They are becoming more international as ID-card signatures spread, currently in use in Estonia and Finland. Why is there 3 of them? ddoc is the original, bdoc the improved and cdoc the improved+encrypted.

      Some more information here: http://id.ee/index.php?id=35780

      The file itself is basically an archive by structure (doc or pdf archived by a signature) and can only be used when downloaded.

      Without these 3 lines, moodle displays the code inside - no use to anyone.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Krister,
            please:

            1. add the upgrade stage too, to fix the already updated files: see here an example;
            2. amend the commit message according with the docs: http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages. Again see the example, where the component could be simply Files instead of Files API as in the example;
            3. double check if application/x-[bcd]doc - used e.g. by qdigidoc - are more appropriate even if application/octet-stream, the default fallback for binary files, seems to work fine.

            TIA,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Hi Krister, please: add the upgrade stage too, to fix the already updated files: see here an example; amend the commit message according with the docs: http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages . Again see the example, where the component could be simply Files instead of Files API as in the example; double check if application/x- [bcd] doc - used e.g. by qdigidoc - are more appropriate even if application/octet-stream , the default fallback for binary files, seems to work fine. TIA, Matteo
            Hide
            kriv Krister Viirsaar added a comment -

            Thanks for the instructions, Matteo. The changes have been implemented.

            1 Done. Though I don't fully understand why this is necessary, so double check the code.
            2a I've made a whole new branch and commited wth better messages.
            2b There is no component "Files", the only related component is "Files API", this is why I've used this.
            3 You are correct. x-ddoc is correct!

            Show
            kriv Krister Viirsaar added a comment - Thanks for the instructions, Matteo. The changes have been implemented. 1 Done. Though I don't fully understand why this is necessary, so double check the code. 2a I've made a whole new branch and commited wth better messages. 2b There is no component "Files", the only related component is "Files API", this is why I've used this. 3 You are correct. x-ddoc is correct!
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for suggesting that and uploading a patch.

            Show
            salvetore Michael de Raadt added a comment - Thanks for suggesting that and uploading a patch.
            Hide
            kriv Krister Viirsaar added a comment -

            bump. Improvement is ready to be integrated.

            Show
            kriv Krister Viirsaar added a comment - bump. Improvement is ready to be integrated.
            Hide
            salvetore Michael de Raadt added a comment -

            Hi, Krister.

            I've added you to the jira-developers group so you can assign this issue to yourself and push it for peer review.

            Show
            salvetore Michael de Raadt added a comment - Hi, Krister. I've added you to the jira-developers group so you can assign this issue to yourself and push it for peer review.
            Hide
            cibot CiBoT added a comment -

            Results for MDL-43570

            Show
            cibot CiBoT added a comment - Results for MDL-43570 Remote repository: https://github.com/KristerV/moodle Remote branch MDL-43570 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/945 Error: The MDL-43570 branch at https://github.com/KristerV/moodle does not apply clean to master
            Hide
            fred Frédéric Massart added a comment -

            Thanks guys for working on this.

            I think this patch is good to go for integration. I had trouble finding some references of the mimetype, so if you could just comment some here that'd be handy.

            Also, could you add some files to the issue that could be used in testing, or inform the tester that fake files would work to.

            Cheers,
            Fred

            Show
            fred Frédéric Massart added a comment - Thanks guys for working on this. I think this patch is good to go for integration. I had trouble finding some references of the mimetype, so if you could just comment some here that'd be handy. Also, could you add some files to the issue that could be used in testing, or inform the tester that fake files would work to. Cheers, Fred
            Hide
            kriv Krister Viirsaar added a comment - - edited

            Ah, alas.. the mimetype is officially x-digidoc. This is the developers website http://www.id.ee/?id=36344
            I've updated my branch to master and also mimetype to x-digidoc.

            Fake files do indeed work, as all it does is check the filename extension.

            May the reviewer please double check the changes in lib/db/upgrade.php

            Show
            kriv Krister Viirsaar added a comment - - edited Ah, alas.. the mimetype is officially x-digidoc. This is the developers website http://www.id.ee/?id=36344 I've updated my branch to master and also mimetype to x-digidoc. Fake files do indeed work, as all it does is check the filename extension. May the reviewer please double check the changes in lib/db/upgrade.php
            Show
            cibot CiBoT added a comment - Results for MDL-43570 Remote repository: https://github.com/KristerV/moodle Remote branch MDL-43570 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1379 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1379/artifact/work/smurf.html
            Hide
            fred Frédéric Massart added a comment - - edited

            Hi Krister,

            This looks good to me now, can I just ask you to:

            1. Provide branches for 2.5 and 2.6 because I think this could be backported.
            2. Add testing instructions to test the upgrade process (upload one of each file before upgrade, upgrade, check that the mimetype has been updated)
            3. Squash your commits as they do not have much value on their own.

            Many thanks!
            Fred

            PS: For integrators, related issue which was backported: MDL-38468

            Show
            fred Frédéric Massart added a comment - - edited Hi Krister, This looks good to me now, can I just ask you to: Provide branches for 2.5 and 2.6 because I think this could be backported. Add testing instructions to test the upgrade process (upload one of each file before upgrade, upgrade, check that the mimetype has been updated) Squash your commits as they do not have much value on their own. Many thanks! Fred PS: For integrators, related issue which was backported: MDL-38468
            Hide
            kriv Krister Viirsaar added a comment -

            1. done
            2. done
            3. done

            Question: This time I did not change the root version.php file, last time I did. I'm pretty sure I'm doing this correctly this time around, but since noone commented on it someone should check it.

            Show
            kriv Krister Viirsaar added a comment - 1. done 2. done 3. done Question: This time I did not change the root version.php file, last time I did. I'm pretty sure I'm doing this correctly this time around, but since noone commented on it someone should check it.
            Show
            cibot CiBoT added a comment - Results for MDL-43570 Remote repository: https://github.com/KristerV/moodle Remote branch MDL-43570 -M25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3206 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/3206/artifact/work/smurf.html Remote branch MDL-43570 -M26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3207 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/3207/artifact/work/smurf.html Remote branch MDL-43570 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3208 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/3208/artifact/work/smurf.html
            Hide
            fred Frédéric Massart added a comment -

            Hi Krister,

            thanks for the changes. I'm afraid that you need to change the root version.php, otherwise the upgrade process does not make sense as it is not bound to any new version. You will also have to use different versions between 2.5, 2.6 and master. Refer to the root version, and slightly increment it. E.g.:

            • From 2013051405.09 to 2013051405.10 for 2.5
            • From 2013111802.09 to 2013111802.10 for 2.6
            • From 2014042400.00 to 2014042401.00 for master

            Each increment will be in version.php and upgrade.php.

            Once this is done, then you are good for integration.

            Many thanks!
            Fred

            Show
            fred Frédéric Massart added a comment - Hi Krister, thanks for the changes. I'm afraid that you need to change the root version.php, otherwise the upgrade process does not make sense as it is not bound to any new version. You will also have to use different versions between 2.5, 2.6 and master. Refer to the root version, and slightly increment it. E.g.: From 2013051405.09 to 2013051405.10 for 2.5 From 2013111802.09 to 2013111802.10 for 2.6 From 2014042400.00 to 2014042401.00 for master Each increment will be in version.php and upgrade.php. Once this is done, then you are good for integration. Many thanks! Fred
            Hide
            kriv Krister Viirsaar added a comment -

            Strange approach. What happens when two different developers integrate an upgrade script at the same time with the same version number?

            But anyway, made the changes, good to go. I don't have integration capability so I'll just ask for peer review.

            Show
            kriv Krister Viirsaar added a comment - Strange approach. What happens when two different developers integrate an upgrade script at the same time with the same version number? But anyway, made the changes, good to go. I don't have integration capability so I'll just ask for peer review.
            Hide
            fred Frédéric Massart added a comment -

            Thanks for fixing those Krister. You are right, it seems strange, but the integrators will catch any conflict with the version number while integrating. Though, it is important to update the version.php so that they notice in your patch that it needs to be looked at.

            I am going to be a tiny bit picky but you do not need to change the release variable, that is done on release only.

            Thank you for your patience and understanding .

            Show
            fred Frédéric Massart added a comment - Thanks for fixing those Krister. You are right, it seems strange, but the integrators will catch any conflict with the version number while integrating. Though, it is important to update the version.php so that they notice in your patch that it needs to be looked at. I am going to be a tiny bit picky but you do not need to change the release variable, that is done on release only. Thank you for your patience and understanding .
            Hide
            kriv Krister Viirsaar added a comment -

            picky is fine, I'll know better next time. Changes done for all three branches!

            Show
            kriv Krister Viirsaar added a comment - picky is fine, I'll know better next time. Changes done for all three branches!
            Hide
            cibot CiBoT added a comment -

            Results for MDL-43570

            Show
            cibot CiBoT added a comment - Results for MDL-43570 Remote repository: https://github.com/KristerV/moodle Remote branch MDL-43570 -M25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3228 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/3228/artifact/work/smurf.html Remote branch MDL-43570 -M26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3229 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/3229/artifact/work/smurf.html Remote branch MDL-43570 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3230 Error: The MDL-43570 branch at https://github.com/KristerV/moodle does not apply clean to master
            Hide
            fred Frédéric Massart added a comment -

            Thanks Krister, pushing for integration now.

            Show
            fred Frédéric Massart added a comment - Thanks Krister, pushing for integration now.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Krister, integrated to master, 26 and 25.

            (We really should add an upgrade lib function for correcting these mime types)

            Show
            poltawski Dan Poltawski added a comment - Thanks Krister, integrated to master, 26 and 25. (We really should add an upgrade lib function for correcting these mime types)
            Hide
            phalacee Jason Fowler added a comment -

            Works fine Krister, thank you for the patch!

            Show
            phalacee Jason Fowler added a comment - Works fine Krister, thank you for the patch!
            Hide
            poltawski Dan Poltawski added a comment -

            Thank you for your contribution! This change is now part of Moodle!

            Nothing is impossible, the word itself says 'I'm possible'!

            --Audrey Hepburn

            Show
            poltawski Dan Poltawski added a comment - Thank you for your contribution! This change is now part of Moodle! Nothing is impossible, the word itself says 'I'm possible'! --Audrey Hepburn

              People

              • Votes:
                2 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/May/14