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

create new PARAM_COMPONENT, PARAM_AREA and PARAM_PLUGIN

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.1, 2.2
    • Fix Version/s: 2.2
    • Component/s: Repositories
    • Labels:
    • Testing Instructions:
      Hide

      This should be a relatively low regression risk, test plugin administration functions + comments, ratings and general file/repository related operations. for example:
      0/ run new unittests in lib/simpletest/testmoodlelib.php
      1/ comments in glossary
      2/ comments in course blocks
      3/ new install and upgrade from 2.0
      4/ installation and uninstallation of blocks, modules and other plugins
      5/ use repositories

      Show
      This should be a relatively low regression risk, test plugin administration functions + comments, ratings and general file/repository related operations. for example: 0/ run new unittests in lib/simpletest/testmoodlelib.php 1/ comments in glossary 2/ comments in course blocks 3/ new install and upgrade from 2.0 4/ installation and uninstallation of blocks, modules and other plugins 5/ use repositories
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w38_MDL-29401_m22_frankenstyle

      Description

      The validation of component, plugin and file areas is very inconsistent, we need new PARAM_COMPONENT, PARAM_PLUGIN and PARAM_AREA.

      • PARAM_COMPONENT - expected to be used for 'frankenstyle' names of plugins, that is plugintype_pluginname
      • PARAM_PLUGIN - plugin name, please note standard modules (mod/*) must not use underscores and numbers are strongly discouraged
      • PARAM_AREA - areas are used in file api, comments, ratings and other subsystems, it is usually used as "contextid + component + area + itemid" which allows content addressing

      Upgrades:

      • core is not affected by the changes
      • contrib code has to be upgraded to use new more restricted format (special chars we already creating problems in different areas)
      • it is not possible to migrate file areas during restore of older backup format yet (see MDL-30930)

        Gliffy Diagrams

          Issue Links

            Activity

            skodak Petr Skoda created issue -
            skodak Petr Skoda made changes -
            Field Original Value New Value
            Assignee Dongsheng Cai [ dongsheng ] Petr Škoda (skodak) [ skodak ]
            skodak Petr Skoda made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            Hide
            skodak Petr Skoda added a comment -

            marking as critical because it may break contrib - this is not a security issue

            Show
            skodak Petr Skoda added a comment - marking as critical because it may break contrib - this is not a security issue
            skodak Petr Skoda made changes -
            Priority Minor [ 4 ] Critical [ 2 ]
            skodak Petr Skoda made changes -
            Summary repositiry cleans filearea and component incorrectly repository cleans filearea and component incorrectly
            skodak Petr Skoda made changes -
            Priority Critical [ 2 ] Major [ 3 ]
            Hide
            skodak Petr Skoda added a comment -

            hmm, it is problematic in many other places - ratings, comments, files
            we should create definitely create PARAM_COMPONENT and PARAM_AREA

            the problem with PARAM_SAFEDIR is the '-' which is not really good, we should also prevent names starting with numbers...

            Show
            skodak Petr Skoda added a comment - hmm, it is problematic in many other places - ratings, comments, files we should create definitely create PARAM_COMPONENT and PARAM_AREA the problem with PARAM_SAFEDIR is the '-' which is not really good, we should also prevent names starting with numbers...
            salvetore Michael de Raadt made changes -
            Fix Version/s DEV backlog [ 10464 ]
            Labels triaged
            nebgor Aparup Banerjee made changes -
            Link This issue has been marked as being related by MDL-26890 [ MDL-26890 ]
            skodak Petr Skoda made changes -
            Summary repository cleans filearea and component incorrectly create new PARAM_COMPONENT, PARAM_AREA and APRAM_PLUGIN
            skodak Petr Skoda made changes -
            Summary create new PARAM_COMPONENT, PARAM_AREA and APRAM_PLUGIN create new PARAM_COMPONENT, PARAM_AREA and PARAM_PLUGIN
            skodak Petr Skoda made changes -
            Link This issue blocks MDL-29504 [ MDL-29504 ]
            skodak Petr Skoda made changes -
            Testing Instructions This should be a relatively low regression risk, test plugin administration functions + comments, ratings and general file/repository related operations. for example:
            1/ comments in glossary
            2/ comments in course blocks
            3/ new install and upgrade from 2.0
            4/ installation and uninstallation of blocks, modules and other plugins
            5/ use repositories
            skodak Petr Skoda made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Pull Master Diff URL https://github.com/skodak/moodle/compare/master...w38_MDL-29401_m22_frankenstyle
            Pull Master Branch w38_MDL-29401_m22_frankenstyle
            Pull from Repository git://github.com/skodak/moodle.git
            Fix Version/s 2.2 [ 10656 ]
            Fix Version/s DEV backlog [ 10464 ]
            Testing Instructions This should be a relatively low regression risk, test plugin administration functions + comments, ratings and general file/repository related operations. for example:
            1/ comments in glossary
            2/ comments in course blocks
            3/ new install and upgrade from 2.0
            4/ installation and uninstallation of blocks, modules and other plugins
            5/ use repositories
            This should be a relatively low regression risk, test plugin administration functions + comments, ratings and general file/repository related operations. for example:
            0/ run new unittests in lib/simpletest/testmoodlelib.php
            1/ comments in glossary
            2/ comments in course blocks
            3/ new install and upgrade from 2.0
            4/ installation and uninstallation of blocks, modules and other plugins
            5/ use repositories
            Hide
            skodak Petr Skoda added a comment -

            the integration request contains new param types:

            • PARAM_COMPONENT - to be used for full component names (aka frankenstyle)
            • PARAM_PLUGIN - to be used for plugin names
            • PARAM_AREA - to be used for file, comment and ratings related areas

            Hopefully this could prevent dev confusion in the future, it may also indirectly improve security especially in 3rd party plugins...

            Show
            skodak Petr Skoda added a comment - the integration request contains new param types: PARAM_COMPONENT - to be used for full component names (aka frankenstyle) PARAM_PLUGIN - to be used for plugin names PARAM_AREA - to be used for file, comment and ratings related areas Hopefully this could prevent dev confusion in the future, it may also indirectly improve security especially in 3rd party plugins...
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            skodak Petr Skoda added a comment -

            oops, Eloy discovered one forgotten PARAM_ALPHAEXT in filestorage class, fixed...

            Show
            skodak Petr Skoda added a comment - oops, Eloy discovered one forgotten PARAM_ALPHAEXT in filestorage class, fixed...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            (note Petr is fixing some bits here, I get this for tomorrow)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - (note Petr is fixing some bits here, I get this for tomorrow)
            stronk7 Eloy Lafuente (stronk7) made changes -
            Integrator samhemelryk stronk7
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, many thanks for all the synchronous support! Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, many thanks for all the synchronous support! Ciao
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Affects Version/s 2.2 [ 10656 ]
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester samhemelryk
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys - tested and passed - have been watching for things while working on testing other issues and havn't seen anything.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks guys - tested and passed - have been watching for things while working on testing other issues and havn't seen anything.
            samhemelryk Sam Hemelryk made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            timhunt Tim Hunt made changes -
            Link This issue has been marked as being related by MDL-29540 [ MDL-29540 ]
            rajeshtaneja Rajesh Taneja made changes -
            Link This issue is blocked by MDL-29542 [ MDL-29542 ]
            rajeshtaneja Rajesh Taneja made changes -
            Link This issue is blocked by MDL-29542 [ MDL-29542 ]
            rajeshtaneja Rajesh Taneja made changes -
            Link This issue caused a regression MDL-29542 [ MDL-29542 ]
            Hide
            nebgor Aparup Banerjee added a comment -

            fixes have been rolled merrily up the stream! Thanks everybody!

            Show
            nebgor Aparup Banerjee added a comment - fixes have been rolled merrily up the stream! Thanks everybody!
            nebgor Aparup Banerjee made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes
            Integration date 28/Sep/11
            skodak Petr Skoda made changes -
            Description oh!
            $filearea = clean_param($params['filearea'], PARAM_ALPHAEXT);
            $component = clean_param($params['component'], PARAM_ALPHAEXT);

            is very wrong, it is already in repository/lib.php:
            1/ unfortuantely we do not have PARAM_FRANKENSTYLE yet for component
            2/ PARAM_SAFEDIR might be best for the filearea


            The validation of component, plugin and file areas is very inconsistent, we need new PARAM_COMPONENT, PARAM_PLUGIN and PARAM_AREA.

            * PARAM_COMPONENT - expected to be used for 'frankenstyle' names of plugins, that is plugintype_pluginname
            * PARAM_PLUGIN - plugin name, please note standard modules (mod/*) must not use underscores and numbers are strongly discouraged
            * PARAM_AREA - areas are used in file api, comments, ratings and other subsystems, it is usually used as "contextid + component + area + itemid" which allows content addressing

            Upgrades:
            * core is not affected by the changes
            * contrib code has to be upgraded to use new more restricted format (special chars we already creating problems in different areas)
            * it is not possible to migrate file areas during restore of older backup format yet (see MDL-30930)
            timhunt Tim Hunt made changes -
            Link This issue caused a regression MDL-31135 [ MDL-31135 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  5/Dec/11