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

          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