Moodle
  1. Moodle
  2. MDL-13761 Issues with add_to_log URL field
  3. MDL-13915

Make add_to_log handle long strings without causing database errors

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.9.1
    • Component/s: Libraries
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      37082

      Description

      This is the changes in MDL-13761 that were agreed with Eloy for probable inclusion in 1.9 branch:

      a) Check length is under the limited for 'url' and 'info' parameters (only ones which are likely to potentially depend on user input) and trim to limited length, with '...', if needed.

      b) If this trim happens, show debugging() info so that developers might notice they are logging something that can get too long.

      c) In log view, if url field ends with ..., don't treat it as a link (as it won't work!).

        Activity

        Hide
        Sam Marshall added a comment -

        Here is a patch which addresses this as described. I have tested this on my MOODLE_19 install (using a wiki with a very long page name), it appears to work.

        Show
        Sam Marshall added a comment - Here is a patch which addresses this as described. I have tested this on my MOODLE_19 install (using a wiki with a very long page name), it appears to work.
        Hide
        Sam Marshall added a comment -

        Petr, I understand you're reviewing all patches for 1.9, could you give me the ok on this one please? thanks

        Show
        Sam Marshall added a comment - Petr, I understand you're reviewing all patches for 1.9, could you give me the ok on this one please? thanks
        Hide
        Petr Škoda added a comment -

        my +1 for commit, just make it DEBUG_DEVELOPER level warning

        Show
        Petr Škoda added a comment - my +1 for commit, just make it DEBUG_DEVELOPER level warning
        Hide
        Petr Škoda added a comment -

        hmm, did you think about unicode?
        strlen() does ascii only, the field is 255 unicode chars, right?

        Show
        Petr Škoda added a comment - hmm, did you think about unicode? strlen() does ascii only, the field is 255 unicode chars, right?
        Hide
        Sam Marshall added a comment -

        Thanks for the catch. Petr was right, I had forgotten about unicode and the previous code could potentially slice a UTF-8 character in half - it also unnecessarily over-restricted the length of strings that included unicode characters.

        I have committed a fix to MOODLE_19 (and HEAD) which uses textlib to do the length checks (and changes debug to debug_developer; for some reason I thought that was default, doh).

        Show
        Sam Marshall added a comment - Thanks for the catch. Petr was right, I had forgotten about unicode and the previous code could potentially slice a UTF-8 character in half - it also unnecessarily over-restricted the length of strings that included unicode characters. I have committed a fix to MOODLE_19 (and HEAD) which uses textlib to do the length checks (and changes debug to debug_developer; for some reason I thought that was default, doh).
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Closing. Thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Closing. Thanks!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: