Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

      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!).

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            quen 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
            quen 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
            quen 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
            quen 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
            skodak Petr Skoda added a comment -

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

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

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

            Show
            skodak Petr Skoda added a comment - hmm, did you think about unicode? strlen() does ascii only, the field is 255 unicode chars, right?
            Hide
            quen 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
            quen 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Closing. Thanks!

            Show
            stronk7 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:
                  Fix Release Date:
                  15/May/08