Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: DEV backlog
    • Component/s: Files API
    • Labels:
    • Affected Branches:
      MOODLE_23_STABLE

      Description

      Make sure that field stored_file::mimetype (and corresponding DB field) is updated each time:
      when file is uploaded, renamed or content is replaced.

      If it is possible to determine the type from the file (binary files have this information inside), use mimetype from inside the file (function finfo_file?). If it is a text file, get mimetype from file extension.

      It is important to display the correct file type in filemanager/filepicker, to determine whether we can display file inline or not, whether we allow or not this type in this particular file area.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              I think there are two issue here.

              1. Check that mimetypes are being recorded correctly - that's a task and for the stable backlog.
              2. Adding a mimetype check - that's an improvement and I think that would need to fall onto the dev backlog.

              I think the second piece of work is the predominant task here. We could split the task, but I see no reason why work on the second task can't involve the first.

              Sorry for being long-winded. I thought I should explain why I triaged this issue this way.

              Also, from my experience, probing a binary file for its mimetype is not always reliable; but perhaps a false negative is better than a false positive.

              Show
              salvetore Michael de Raadt added a comment - I think there are two issue here. Check that mimetypes are being recorded correctly - that's a task and for the stable backlog. Adding a mimetype check - that's an improvement and I think that would need to fall onto the dev backlog. I think the second piece of work is the predominant task here. We could split the task, but I see no reason why work on the second task can't involve the first. Sorry for being long-winded. I thought I should explain why I triaged this issue this way. Also, from my experience, probing a binary file for its mimetype is not always reliable; but perhaps a false negative is better than a false positive.
              Hide
              dongsheng Dongsheng Cai added a comment -
              Show
              dongsheng Dongsheng Cai added a comment - Is this what you need? https://github.com/dongsheng/moodle/commit/a86eb11
              Hide
              marina Marina Glancy added a comment -

              Dongsheng, I suppose line 1155 should be:

              $filepathname = $this->path_from_hash($newrecord->contenthash).'/'.$newrecord->contenthash;

              also line 1225 and 1338 are strange - why do we address a file just by filename, without the full path?

              And is stored_file:
              simple call get_content_file_location() without try_content_recovery() will not work in some cases.

              Show
              marina Marina Glancy added a comment - Dongsheng, I suppose line 1155 should be: $filepathname = $this->path_from_hash($newrecord->contenthash).'/'.$newrecord->contenthash; also line 1225 and 1338 are strange - why do we address a file just by filename, without the full path? And is stored_file: simple call get_content_file_location() without try_content_recovery() will not work in some cases.
              Hide
              marina Marina Glancy added a comment -

              file_storage::convert_image() - why don't you use $imageinfo['mimetype'] here ?

              Show
              marina Marina Glancy added a comment - file_storage::convert_image() - why don't you use $imageinfo ['mimetype'] here ?
              Hide
              marina Marina Glancy added a comment -

              I put your commit into wip-files23

              I can't understand something
              there is file without extension in private files
              it is displayed as 'Text file' in filemanager and 'File' in file picker
              something with mimetype, not sure why it happens

              Show
              marina Marina Glancy added a comment - I put your commit into wip-files23 I can't understand something there is file without extension in private files it is displayed as 'Text file' in filemanager and 'File' in file picker something with mimetype, not sure why it happens
              Hide
              marina Marina Glancy added a comment -
              Show
              marina Marina Glancy added a comment - maybe this is the reason: https://github.com/marinaglancy/moodle/blob/wip-files23/lib/filebrowser/file_info_stored.php#L486 the mimetype in file is ignored here
              Hide
              marina Marina Glancy added a comment -

              fixed in wip-files23 branch

              Show
              marina Marina Glancy added a comment - fixed in wip-files23 branch
              Hide
              timhunt Tim Hunt added a comment -

              I'm not sure this is really an improvement. See http://tracker.moodle.org/browse/MDL-34738?focusedCommentId=174652&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-174652 and following comments.

              We are uploading a .dat file. The file system things that is text/plain, which is a pretty poor guess. application/xml might, possibly, be an acceptable guess.

              Any suggestions for fixing the problem in MDL-34738 would be welcome.

              Show
              timhunt Tim Hunt added a comment - I'm not sure this is really an improvement. See http://tracker.moodle.org/browse/MDL-34738?focusedCommentId=174652&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-174652 and following comments. We are uploading a .dat file. The file system things that is text/plain, which is a pretty poor guess. application/xml might, possibly, be an acceptable guess. Any suggestions for fixing the problem in MDL-34738 would be welcome.

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved: