Moodle
  1. Moodle
  2. MDL-29401

create new PARAM_COMPONENT, PARAM_AREA and PARAM_PLUGIN

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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

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

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

            Show
            Petr Skoda added a comment - marking as critical because it may break contrib - this is not a security issue
            Petr Skoda made changes -
            Priority Minor [ 4 ] Critical [ 2 ]
            Petr Skoda made changes -
            Summary repositiry cleans filearea and component incorrectly repository cleans filearea and component incorrectly
            Petr Skoda made changes -
            Priority Critical [ 2 ] Major [ 3 ]
            Hide
            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
            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...
            Michael de Raadt made changes -
            Fix Version/s DEV backlog [ 10464 ]
            Labels triaged
            Aparup Banerjee made changes -
            Link This issue has been marked as being related by MDL-26890 [ MDL-26890 ]
            Petr Skoda made changes -
            Summary repository cleans filearea and component incorrectly create new PARAM_COMPONENT, PARAM_AREA and APRAM_PLUGIN
            Petr Skoda made changes -
            Summary create new PARAM_COMPONENT, PARAM_AREA and APRAM_PLUGIN create new PARAM_COMPONENT, PARAM_AREA and PARAM_PLUGIN
            Petr Skoda made changes -
            Link This issue blocks MDL-29504 [ MDL-29504 ]
            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
            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
            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
            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...
            Sam Hemelryk made changes -
            Currently in integration Yes
            Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            Petr Skoda added a comment -

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

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

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

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

            Integrated, many thanks for all the synchronous support! Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Integrated, many thanks for all the synchronous support! Ciao
            Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Affects Version/s 2.2 [ 10656 ]
            Sam Hemelryk made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester samhemelryk
            Hide
            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
            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.
            Sam Hemelryk made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Tim Hunt made changes -
            Link This issue has been marked as being related by MDL-29540 [ MDL-29540 ]
            Rajesh Taneja made changes -
            Link This issue is blocked by MDL-29542 [ MDL-29542 ]
            Rajesh Taneja made changes -
            Link This issue is blocked by MDL-29542 [ MDL-29542 ]
            Rajesh Taneja made changes -
            Link This issue caused a regression MDL-29542 [ MDL-29542 ]
            Hide
            Aparup Banerjee added a comment -

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

            Show
            Aparup Banerjee added a comment - fixes have been rolled merrily up the stream! Thanks everybody!
            Aparup Banerjee made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes
            Integration date 28/Sep/11
            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)
            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: