Moodle
  1. Moodle
  2. MDL-33353

detect buggy iconv() with //IGNORE and use mbstring if exists or warn admins (backport of MDL-32586 and MDL-33007)

    Details

    • Testing Instructions:
      Hide

      This is about to execute lib/simpletest/testmoodlelib.php under certain conditions:

      1- TEST: unit tests pass under a "good" system (MacOS using mac ports LAMP stack, like the laptop integration server).

      2- TEST: unit tests pass under a "glibc" system (debian, ubuntu, like the "integrationci" integration server).

      3- TEST: unit tests pass under a "bad" system (redhat? or, alrernatively, hack fix_utf8() and set $buggyiconv = true, so fallback to mbstring will be used if available.

      4- TEST: unit test fail in a "completely borked" system. With the hack in previous point in action, disable mbstring (or hack the function_exists('mb_convert_encoding') check).

      Show
      This is about to execute lib/simpletest/testmoodlelib.php under certain conditions: 1- TEST: unit tests pass under a "good" system (MacOS using mac ports LAMP stack, like the laptop integration server). 2- TEST: unit tests pass under a "glibc" system (debian, ubuntu, like the "integrationci" integration server). 3- TEST: unit tests pass under a "bad" system (redhat? or, alrernatively, hack fix_utf8() and set $buggyiconv = true, so fallback to mbstring will be used if available. 4- TEST: unit test fail in a "completely borked" system. With the hack in previous point in action, disable mbstring (or hack the function_exists('mb_convert_encoding') check).
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Rank (Obsolete):
      41250

      Description

      This is a followup of MDL-32586 and MDL-33007, aimed to:

      • Improve fix_utf8() to avoid some annoying notices and provide mbstring() alternative for buggy iconv libraries (not supporting //IGNORE).
      • Inform the admin (in notifications page and/or environmental checks, to decide) about the site being run using a buggy iconv without mbstring alternative.

      To backport to 22 and 21, ideally, once the original issues have been tested in a bunch of OS/PHP combinations. I think this will make life really better for a lot of debian/ubuntu/redhat sites.

      Ciao

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          The plan is to backport the changes performed to fix_utf8() by MDL-32586 & MDL-33007 to STABLE branches.

          That will cause, automatically, the function to work better under some buggy systems and also, tests to pass under those systems.

          This won't include all the admin notifications stuff, nor the min_fix_utf8(), that's new for Moodle 2.3. Only the improved handling of buggy situations within fix_utf8().

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The plan is to backport the changes performed to fix_utf8() by MDL-32586 & MDL-33007 to STABLE branches. That will cause, automatically, the function to work better under some buggy systems and also, tests to pass under those systems. This won't include all the admin notifications stuff, nor the min_fix_utf8(), that's new for Moodle 2.3. Only the improved handling of buggy situations within fix_utf8(). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sending to integration, 22_STABLE only.

          Show
          Eloy Lafuente (stronk7) added a comment - Sending to integration, 22_STABLE only.
          Hide
          Sam Hemelryk added a comment -

          Thanks Eloy, changes look spot on and have been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Eloy, changes look spot on and have been integrated now.
          Hide
          Dan Poltawski added a comment -

          This might not be related at all, but on my centos install i'm still getting:

          2) grading_manager_testcase::test_tokenize
          A test using UTF-8 characters has failed. Consider updating PHP and PHP's PCRE or INTL extensions (MDL-30494)
          Failed asserting that false is true.

          /server/workspace/moodle/grade/grading/tests/lib_test.php:105
          /server/workspace/moodle/lib/phpunit/classes/advanced_testcase.php:76

          To re-run:
          phpunit grading_manager_testcase grade/grading/tests/lib_test.php

          Show
          Dan Poltawski added a comment - This might not be related at all, but on my centos install i'm still getting: 2) grading_manager_testcase::test_tokenize A test using UTF-8 characters has failed. Consider updating PHP and PHP's PCRE or INTL extensions ( MDL-30494 ) Failed asserting that false is true. /server/workspace/moodle/grade/grading/tests/lib_test.php:105 /server/workspace/moodle/lib/phpunit/classes/advanced_testcase.php:76 To re-run: phpunit grading_manager_testcase grade/grading/tests/lib_test.php
          Hide
          Dan Poltawski added a comment -

          I don't have this handy: '2- TEST: unit tests pass under a "glibc" system (debian, ubuntu, like the "integrationci" integration server).'

          But I think its been passed by integrationci now anyway, correct?

          Show
          Dan Poltawski added a comment - I don't have this handy: '2- TEST: unit tests pass under a "glibc" system (debian, ubuntu, like the "integrationci" integration server).' But I think its been passed by integrationci now anyway, correct?
          Hide
          Dan Poltawski added a comment -

          Centos, before patch:
          test_fix_utf8
          Unexpected PHP error [iconv(): Detected an illegal character in input string] severity [E_NOTICE] in [/server/workspace/moodle/lib/moodlelib.php line 1130]

          Show
          Dan Poltawski added a comment - Centos, before patch: test_fix_utf8 Unexpected PHP error [iconv(): Detected an illegal character in input string] severity [E_NOTICE] in [/server/workspace/moodle/lib/moodlelib.php line 1130]
          Hide
          Dan Poltawski added a comment -

          After, passes! :-D

          Show
          Dan Poltawski added a comment - After, passes! :-D
          Hide
          Dan Poltawski added a comment -

          Damn, actually I ddin't have the 'bad' implemention. However hacking it and everything worked as expected.

          Show
          Dan Poltawski added a comment - Damn, actually I ddin't have the 'bad' implemention. However hacking it and everything worked as expected.
          Hide
          Dan Poltawski added a comment -

          Passing. Thanks.

          Show
          Dan Poltawski added a comment - Passing. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

          Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

          Many thanks for your collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: