Moodle
  1. Moodle
  2. MDL-34952

MDL-29804 causes issues while trying to read get_info from external class

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.4, 2.3.1, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Feedback
    • Labels:
    • Testing Instructions:
      Hide

      There is nothing to test. It is just for a public access to the function get_info() in multichoice/lib.php and multichoicerated/lib.php

      Show
      There is nothing to test. It is just for a public access to the function get_info() in multichoice/lib.php and multichoicerated/lib.php
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34952_master
    • Rank:
      43520

      Description

      We are interfacing with the Feedback module from external systems allowing us to generate reports stored outside of the context of Moodle.
      To accomplish the generating part, we used to use get_info, however, this is protected now by MDL-29804.
      Would it be possible to change this to public again in the case of /mod/feedback/item/multi*/lib.php?

        Activity

        Hide
        Michael de Raadt added a comment -

        Up to you, Andreas.

        Show
        Michael de Raadt added a comment - Up to you, Andreas.
        Hide
        Andreas Grabs added a comment -

        Hi Sebastian,

        I think there shouldn't be any problem with public access. So I changed it.

        Best regards
        Andreas

        Show
        Andreas Grabs added a comment - Hi Sebastian, I think there shouldn't be any problem with public access. So I changed it. Best regards Andreas
        Hide
        Aparup Banerjee added a comment - - edited

        Hm its a trivial change but just not sure if we should backport private->public changes to stable branches, i believe there may be probable security concerns for others (overiding classes get too much access?).
        but i may be too paranoid here (but just being cautious to throwing the question), there may be nothing to worry about here.. thoughts anyone?

        edit: now less concerned - its jsut info being available - no writing..

        Show
        Aparup Banerjee added a comment - - edited Hm its a trivial change but just not sure if we should backport private->public changes to stable branches, i believe there may be probable security concerns for others (overiding classes get too much access?). but i may be too paranoid here (but just being cautious to throwing the question), there may be nothing to worry about here.. thoughts anyone? edit: now less concerned - its jsut info being available - no writing..
        Hide
        Sebastian Berm added a comment -

        Hi Agarup,

        I do agree... We should all (including the developers of third party modules) take care we don't allow write access to parts where there can be problems with this. In this case it's just read only access. Maybe it's wise to set up a guideline for this in the developers wiki?
        If it would be possible for the original creator of the issue to follow a set of tests or anything before even being able to add the issue, it might save some time in the future?

        Best regards,
        Sebastian

        Show
        Sebastian Berm added a comment - Hi Agarup, I do agree... We should all (including the developers of third party modules) take care we don't allow write access to parts where there can be problems with this. In this case it's just read only access. Maybe it's wise to set up a guideline for this in the developers wiki? If it would be possible for the original creator of the issue to follow a set of tests or anything before even being able to add the issue, it might save some time in the future? Best regards, Sebastian
        Hide
        Andreas Grabs added a comment -

        Hi,

        I do not see any security reason if there is public access to a method.
        If an contributor really need this feature then he/she would write the own code to have this feature or copy the private method. And that in fact could be a security issue. We can not maintain code written by the contributor.
        In my opinion the public/protected/private etc. access modifiers help us to encapsulate the internals of a class.
        But if a feature is needed by other objects so it should be public.
        Anybody who has access to the code can write what he want. I can not see how a private access modifier can prevent this.
        Perhaps I'm totally wrong.

        Best regards
        Andreas

        Show
        Andreas Grabs added a comment - Hi, I do not see any security reason if there is public access to a method. If an contributor really need this feature then he/she would write the own code to have this feature or copy the private method. And that in fact could be a security issue. We can not maintain code written by the contributor. In my opinion the public/protected/private etc. access modifiers help us to encapsulate the internals of a class. But if a feature is needed by other objects so it should be public. Anybody who has access to the code can write what he want. I can not see how a private access modifier can prevent this. Perhaps I'm totally wrong. Best regards Andreas
        Hide
        Aparup Banerjee added a comment -

        Thanks for the discussion. (perhaps any further guidelines needed can be raised in

        Its integrated into master now.

        Show
        Aparup Banerjee added a comment - Thanks for the discussion. (perhaps any further guidelines needed can be raised in Its integrated into master now.
        Hide
        Tim Barker added a comment -

        See the testing instructions no testing is required.

        Show
        Tim Barker added a comment - See the testing instructions no testing is required.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I'm so proud...of you, many thanks!

        http://youtu.be/n64CdfDRnZY

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - I'm so proud...of you, many thanks! http://youtu.be/n64CdfDRnZY Closing as fixed, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: