Moodle
  1. Moodle
  2. MDL-29773

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical 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
    • Rank:
      19288

      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]

        Activity

        Hide
        Andrea Bicciolo added a comment -

        Assigning to Petr

        Show
        Andrea Bicciolo added a comment - Assigning to Petr
        Hide
        Petr Škoda 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
        Petr Škoda 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
        Petr Škoda added a comment -

        Please cherry pick to all 2.x branches.

        Show
        Petr Škoda added a comment - Please cherry pick to all 2.x branches.
        Hide
        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
        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
        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
        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
        Petr Škoda 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
        Petr Škoda 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
        Petr Škoda 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
        Petr Škoda 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
        Petr Škoda 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
        Petr Škoda 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
        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
        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
        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
        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: