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
    • Rank:
      18955

      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)

        Issue Links

          Activity

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

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

          Show
          Petr Škoda added a comment - marking as critical because it may break contrib - this is not a security issue
          Petr Škoda made changes -
          Priority Minor [ 4 ] Critical [ 2 ]
          Petr Škoda made changes -
          Summary repositiry cleans filearea and component incorrectly repository cleans filearea and component incorrectly
          Petr Škoda made changes -
          Priority Critical [ 2 ] Major [ 3 ]
          Hide
          Petr Škoda 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 Škoda 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 Škoda made changes -
          Summary repository cleans filearea and component incorrectly create new PARAM_COMPONENT, PARAM_AREA and APRAM_PLUGIN
          Petr Škoda made changes -
          Summary create new PARAM_COMPONENT, PARAM_AREA and APRAM_PLUGIN create new PARAM_COMPONENT, PARAM_AREA and PARAM_PLUGIN
          Petr Škoda made changes -
          Link This issue blocks MDL-29504 [ MDL-29504 ]
          Petr Škoda 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 Škoda 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 Škoda 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 Škoda 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 Škoda added a comment -

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

          Show
          Petr Škoda 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 Škoda 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: