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

Upgrade to moodle 2.1 fails if files time stamp is before 1 january 1970

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.5, 2.1.2, 2.2
    • Fix Version/s: 2.0.6, 2.1.3
    • Component/s: Administration
    • Labels:
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      find 3 places that create new files from:
      1/ string (some export)
      2/ existing file (pick from local repo)
      3/ from fielsystem file (upload repo)

      Somehow verify the file dates are correct. I suppose there is not place in moodle UI where you could see the dates, right? So probably just use some db browser tool.

      Show
      find 3 places that create new files from: 1/ string (some export) 2/ existing file (pick from local repo) 3/ from fielsystem file (upload repo) Somehow verify the file dates are correct. I suppose there is not place in moodle UI where you could see the dates, right? So probably just use some db browser tool.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
      git@github.com:skodak/moodle.git
    • Pull Master Branch:
      w41_MDL-29773_m22_filedates

      Description

      When migrating to Moodle 2.1, upgrade fails if file have time stamp before 1 January 1970. This problem has been discussed with Petr with enhanced file_storage debug, and the following is the result when dealing with before 1970 files in mod_scorm:

      !!! Can not create file "838/mod_scorm/content/0//common//anyElement.xsd" !!!
      !! Out of range value for column 'timemodified' at row 1
      INSERT INTO mdl_files (contextid,component,filearea,itemid,filepath,filename,timecreated,timemodified,mimetype,userid,source,author,license,sortorder,filesize, contenthash,pathnamehash) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
      [array (
      0 => '838',
      1 => 'mod_scorm',
      2 => 'content',
      3 => 0,
      4 => '/common/',
      5 => 'anyElement.xsd',
      6 => 1318499551,
      7 => -426241696,
      8 => 'document/unknown',
      9 => NULL,
      10 => NULL,
      11 => NULL,
      12 => NULL,
      13 => 0,
      14 => 1016,
      15 => 'af73428a4d36730ac46eaddd4a4984e8f4c3d6c1',
      16 => 'cba7933a180d03bdd509e3538423e9324e21e831',
      )] !!
      !! Stack trace: * line 826 of /lib/filestorage/file_storage.php: stored_file_creation_exception thrown

      • line 93 of /mod/scorm/db/upgradelib.php: call to file_storage->create_file_from_pathname()
      • line 105 of /mod/scorm/db/upgradelib.php: call to scorm_migrate_moddata_subdir()
      • line 44 of /mod/scorm/db/upgradelib.php: call to scorm_migrate_moddata_subdir()
      • line 177 of /mod/scorm/db/upgrade.php: call to scorm_migrate_moddata_files()
      • line 541 of /lib/upgradelib.php: call to xmldb_scorm_upgrade()
      • line 271 of /lib/upgradelib.php: call to upgrade_plugins_modules()
      • line 1466 of /lib/upgradelib.php: call to upgrade_plugins()
      • line 146 of /admin/cli/upgrade.php: call to upgrade_noncore()
        !!
        [Exit 1]

        Gliffy Diagrams

          Activity

          Hide
          andreabix Andrea Bicciolo added a comment -

          Assigning to Petr

          Show
          andreabix Andrea Bicciolo added a comment - Assigning to Petr
          Hide
          skodak Petr Skoda added a comment -

          my plan is to add the missing debug that produced the stacktrace above and set the dates to non-negative timestamps. IN the future I hope we get rid of all unsigned ints in database...

          Show
          skodak Petr Skoda added a comment - my plan is to add the missing debug that produced the stacktrace above and set the dates to non-negative timestamps. IN the future I hope we get rid of all unsigned ints in database...
          Hide
          skodak Petr Skoda added a comment -

          Please cherry pick to all 2.x branches.

          Show
          skodak Petr Skoda added a comment - Please cherry pick to all 2.x branches.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Hi, I've integrated this for master, 20 and 21 stables.

          My only concern is that, after applying the patch, one new file_exception() is thrown when timecreated|timemodified is, for example, null or false (because of the is_number new check) and this wasn't happening before the patch.

          Could this affect anything in upgrade/core? If so, perhaps we should make that check a bit more explicit? Just to confirm.

          Andrea, could you test this fix against your broken site and confirm that you get 0 (January 1970) in your offending files? TIA!

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Hi, I've integrated this for master, 20 and 21 stables. My only concern is that, after applying the patch, one new file_exception() is thrown when timecreated|timemodified is, for example, null or false (because of the is_number new check) and this wasn't happening before the patch. Could this affect anything in upgrade/core? If so, perhaps we should make that check a bit more explicit? Just to confirm. Andrea, could you test this fix against your broken site and confirm that you get 0 (January 1970) in your offending files? TIA! Ciao
          Hide
          nebgor Aparup Banerjee added a comment - - edited

          i'm not sure how to pass this as its not too clear for me about the strings:

          I used grade export to export a file created from strings - where is the DB entry? mdl_files doesn't have anything.

          otherwise,
          picking existing file and uploading new file seems to create correct timestamps for me.

          Show
          nebgor Aparup Banerjee added a comment - - edited i'm not sure how to pass this as its not too clear for me about the strings: I used grade export to export a file created from strings - where is the DB entry? mdl_files doesn't have anything. otherwise, picking existing file and uploading new file seems to create correct timestamps for me.
          Hide
          skodak Petr Skoda added a comment -

          The testing should confirm the that the file creation works as before for positive date timestamps, the fix was already confirmed to fix the problem with negative dates.

          Show
          skodak Petr Skoda added a comment - The testing should confirm the that the file creation works as before for positive date timestamps, the fix was already confirmed to fix the problem with negative dates.
          Hide
          skodak Petr Skoda added a comment -

          Eloy: yes, the exception would be surprising for people that use invalid timestamps, I do not think we should silently cast them to something valid. Core code is using file timestamps that should be valid integers, FALSE could be there from http://cz.php.net/manual/en/function.filemtime.php and friends if they can not read the dates somehow.

          Show
          skodak Petr Skoda added a comment - Eloy: yes, the exception would be surprising for people that use invalid timestamps, I do not think we should silently cast them to something valid. Core code is using file timestamps that should be valid integers, FALSE could be there from http://cz.php.net/manual/en/function.filemtime.php and friends if they can not read the dates somehow.
          Hide
          skodak Petr Skoda added a comment -

          Oh, Andrea confirmed the initial patch fixed the problem for him, so we are 100% sure the problem was caused by invalid date before 1970.

          Show
          skodak Petr Skoda added a comment - Oh, Andrea confirmed the initial patch fixed the problem for him, so we are 100% sure the problem was caused by invalid date before 1970.
          Hide
          nebgor Aparup Banerjee added a comment -

          ok i've tested with upgrading and issue seems resolved. negative timestamps ==> 0

          also seems theres been discussion somewhere which makes this easier to pass.

          Show
          nebgor Aparup Banerjee added a comment - ok i've tested with upgrading and issue seems resolved. negative timestamps ==> 0 also seems theres been discussion somewhere which makes this easier to pass.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Many thanks for all the hard work. This is now part of Moodle, your favorite LMS.

          Closing as fixed, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for all the hard work. This is now part of Moodle, your favorite LMS. Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                28/Nov/11