Moodle
  1. Moodle
  2. MDL-31514

Time format causing error messages on Windows servers

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Libraries
    • Labels:
    • Environment:
      Win 7, XAMPP, MySQL, any browser
    • Rank:
      38062

      Description

      The following error is being reported numerous times around the site.

      Warning: Invalid CRT parameters detected in D:\xampp\htdocs\moodle_master\lib\moodlelib.php on line 1975
      

      This is caused by a change made in MDL-6102 in an attempt to remove leading zeros from time values. The format sequence %I was replaced with %l (lower-case L) in lang/en/langconfig.php, however this is not recognised by the libraries used to power Apache under Windows.

      I'm not sure why this change was made. The userdate() function already strips leading zeros, so this change would have had no effect. Also the added stripping of double-spaces in userdate(), added with the fix in MDL-6102 is probably not needed.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          I've just added a patch, but essentially this just reverses MDL-6102.

          Show
          Michael de Raadt added a comment - I've just added a patch, but essentially this just reverses MDL-6102 .
          Hide
          Sam Marshall added a comment -

          Oops. I'll do a proper commit for this (although if you want to just revert it as an emergency measure, that would be OK).

          Leading zeros were not suppressed before the change. The code in userdate removes leading zeros from the 'day' field (%d) but not from the 'hour' field (%I).

          Show
          Sam Marshall added a comment - Oops. I'll do a proper commit for this (although if you want to just revert it as an emergency measure, that would be OK). Leading zeros were not suppressed before the change. The code in userdate removes leading zeros from the 'day' field (%d) but not from the 'hour' field (%I).
          Hide
          Sam Marshall added a comment -

          Glad it was only done for master...

          This fix combines the following:

          a) revert previous patch (done with git revert)
          b) add logic similar to the 'fixday' parameter so that it can also fix the hour parameter (called 'fixhour')

          Show
          Sam Marshall added a comment - Glad it was only done for master... This fix combines the following: a) revert previous patch (done with git revert) b) add logic similar to the 'fixday' parameter so that it can also fix the hour parameter (called 'fixhour')
          Hide
          Michael de Raadt added a comment -

          Thanks for working on that, Sam.

          That solution looks good. Pushing this to integration.

          Show
          Michael de Raadt added a comment - Thanks for working on that, Sam. That solution looks good. Pushing this to integration.
          Hide
          Sam Hemelryk added a comment -

          Hi Sam,
          I'm sending this back at the moment, changes appear to be 100% on target however the unit tests for userdate need to be updated to reflect these changes.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Sam, I'm sending this back at the moment, changes appear to be 100% on target however the unit tests for userdate need to be updated to reflect these changes. Cheers Sam
          Hide
          Michael de Raadt added a comment -

          Sorry, Sam and Sam.

          I should have spotted that. Very tardy of me. I'm just keen to see this fix integrated because it's really annoying me.

          Show
          Michael de Raadt added a comment - Sorry, Sam and Sam. I should have spotted that. Very tardy of me. I'm just keen to see this fix integrated because it's really annoying me.
          Hide
          Sam Marshall added a comment -

          Yes sorry - will update unit tests today and resubmit.

          Show
          Sam Marshall added a comment - Yes sorry - will update unit tests today and resubmit.
          Hide
          Sam Marshall added a comment -

          Sorry, I have now corrected the unit tests (in the expected way i.e. I changed all the examples from 07:00 AM to 7:00 AM) and they are now working. (With the exception that, unrelated to this code, there's one failure about UTF-8 conversion or something.)

          Note: The bug is marked as affects stable etc but I think it was only ever broken on master (2.3 dev).

          Show
          Sam Marshall added a comment - Sorry, I have now corrected the unit tests (in the expected way i.e. I changed all the examples from 07:00 AM to 7:00 AM) and they are now working. (With the exception that, unrelated to this code, there's one failure about UTF-8 conversion or something.) Note: The bug is marked as affects stable etc but I think it was only ever broken on master (2.3 dev).
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Sam Marshall added a comment -

          Rebased.

          Show
          Sam Marshall added a comment - Rebased.
          Hide
          Sam Hemelryk added a comment -

          Thanks Sam, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Sam, this has been integrated now
          Hide
          Ankit Agarwal added a comment -

          Nice to see get this integrated
          Everything works as expected. I found an unrelated issue in recent activity page.
          Created MDL-31734 to deal with it.
          Thanks for fixing this
          passing

          Show
          Ankit Agarwal added a comment - Nice to see get this integrated Everything works as expected. I found an unrelated issue in recent activity page. Created MDL-31734 to deal with it. Thanks for fixing this passing
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Some changes to Moodle should be milestones in the project by themselves.

          This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: