Moodle
  1. Moodle
  2. MDL-23335

Multi-lang filtering broken at Frontpage News forum display

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.9, 2.0, 2.4, 2.4.6, 2.5.2
    • Fix Version/s: 2.4.7, 2.5.3
    • Component/s: Forum
    • Labels:
    • Testing Instructions:
      Hide
      1. Log in as admin
      2. Navigate to Site admin > Plugins > Filters > Manage filters
      3. Turn the Multi-Language Content filter to On
      4. Navigate to Site admin > Language > Language packs
      5. Ensure the English (en) and Français (fr) language packs are installed
      6. Navigate to Site admin > Front page > Front page settings
      7. For the Front page items when logged in settings, add News items
      8. Navigate to Home
      9. Under Site news, click Add a new topic
      10. Add an arbitrary title
      11. In the editor for the content, click the HTML button
      12. Paste the following content
        <p><span class="multilang" lang="en">English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english.</span><span class="multilang" lang="fr">French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. </span></p>
        
      13. Click Update
      14. Click Save changes
      15. Click Home and wait until the page is fully loaded
      16. Switch languages between English and French
      17. VERIFY that the appropriate message appears in each language
      Show
      Log in as admin Navigate to Site admin > Plugins > Filters > Manage filters Turn the Multi-Language Content filter to On Navigate to Site admin > Language > Language packs Ensure the English (en) and Français (fr) language packs are installed Navigate to Site admin > Front page > Front page settings For the Front page items when logged in settings, add News items Navigate to Home Under Site news, click Add a new topic Add an arbitrary title In the editor for the content, click the HTML button Paste the following content <p><span class="multilang" lang="en">English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english.</span><span class="multilang" lang="fr">French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. </span></p> Click Update Click Save changes Click Home and wait until the page is fully loaded Switch languages between English and French VERIFY that the appropriate message appears in each language
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      wip-mdl-23335-m24
    • Pull 2.5 Branch:
      wip-mdl-23335-m25
    • Pull Master Branch:
      wip-mdl-23335-master
    • Rank:
      6144

      Description

      Multi-lang filtering is broken on the Frontpage News forum display. The problem is that the forum content is being shortened by forum_shorten_post() before the multi-lang filter can be applied through format_text().

      The current code is...

      mod/forum/lib.php ~line 3482
              $postcontent  = format_text(forum_shorten_post($post->message), $post->messageformat, $options, $course->id);
      

      An untested possible solution is change the order of format_text and forum_shorten_post functions:

      mod/forum/lib.php ~line 3482
              $postcontent  = forum_shorten_post(format_text($post->message, $post->messageformat, $options, $course->id));
      

      Replication steps:

      1. Log in as admin
      2. Navigate to Site admin > Plugins > Filters > Manage filters
      3. Turn the Multi-Language Content filter to On
      4. Navigate to Site admin > Language > Language packs
      5. Ensure the English (en) and Français (fr) language packs are installed
      6. Navigate to Site admin > Front page > Front page settings
      7. For the Front page items when logged in settings, add News items
      8. Navigate to Home
      9. Under Site news, click Add a new topic
      10. Add an arbitrary title
      11. In the editor for the content, click the HTML button
      12. Paste the following content
        <p><span class="multilang" lang="en">English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english. English english english english.</span><span class="multilang" lang="fr">French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. French french french french. </span></p>
        
      13. Click Update
      14. Click Save changes
      15. Click Home
      16. Switch languages between English and French

      Expected result: The shortened message in the appropriate language should be shown

      Actual result: The first language in the content (English) is always shown.

        Issue Links

          Activity

          Hide
          Juan Leyva (CV&A) added a comment -

          This happens when the FRONTPAGE NEWS FORUM is showed in the frontpage.

          Example text:

          <span class="multilang" lang="en">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam venenatis fringilla quam, id fermentum sem tincidunt vel. Donec neque velit, pulvinar a gravida sed, consectetur id neque. Morbi a nulla elit, nec eleifend justo. Nulla lacus quam, vestibulum et faucibus a, tempor in leo. Aenean in nunc non neque vestibulum auctor. Cras non sapien nisi, eget molestie augue. Donec elementum vehicula ante, ut imperdiet urna vestibulum vitae. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae;</span><span class="multilang" lang="es">Texto de prueba para probar que los foros cortan mal el texto, a continuación bla bla bla blabla bla bla blabla bla bla bla bla bla bla blabla bla bla blabla bla bla blabla bla bla bla bla bla bla blabla bla bla blabla bla bla bla bla bla bla blabla bla bla blabla bla bla blabla bla bla blabla bla bla blabla bla bla bla </span>

          Show
          Juan Leyva (CV&A) added a comment - This happens when the FRONTPAGE NEWS FORUM is showed in the frontpage. Example text: <span class="multilang" lang="en">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam venenatis fringilla quam, id fermentum sem tincidunt vel. Donec neque velit, pulvinar a gravida sed, consectetur id neque. Morbi a nulla elit, nec eleifend justo. Nulla lacus quam, vestibulum et faucibus a, tempor in leo. Aenean in nunc non neque vestibulum auctor. Cras non sapien nisi, eget molestie augue. Donec elementum vehicula ante, ut imperdiet urna vestibulum vitae. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae;</span><span class="multilang" lang="es">Texto de prueba para probar que los foros cortan mal el texto, a continuación bla bla bla blabla bla bla blabla bla bla bla bla bla bla blabla bla bla blabla bla bla blabla bla bla bla bla bla bla blabla bla bla blabla bla bla bla bla bla bla blabla bla bla blabla bla bla blabla bla bla blabla bla bla blabla bla bla bla </span>
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this. It still seems to be a problem.

          I tried switching the order of the two functions as you suggested. It worked in allowing the correct language text to be shown, but there was an line break lost. I'll see if I can come up with a better solution.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. It still seems to be a problem. I tried switching the order of the two functions as you suggested. It worked in allowing the correct language text to be shown, but there was an line break lost. I'll see if I can come up with a better solution.
          Hide
          Michael de Raadt added a comment -

          Your suggested fix was correct for getting the multi-lang filter to work, but the forum_shorten_text() function was truncating text with disregard to open tags. I've replaced that function's content with a call to shorten_text(), which does handle tags appropriately.

          Show
          Michael de Raadt added a comment - Your suggested fix was correct for getting the multi-lang filter to work, but the forum_shorten_text() function was truncating text with disregard to open tags. I've replaced that function's content with a call to shorten_text(), which does handle tags appropriately.
          Hide
          Michael de Raadt added a comment -

          I checked Moodle versions back to 2.3 and this code has not changed since then, so this fix should be cherry-pickable.

          Show
          Michael de Raadt added a comment - I checked Moodle versions back to 2.3 and this code has not changed since then, so this fix should be cherry-pickable.
          Hide
          Mark Nelson added a comment -

          Hi Michael,

          One issue I spotted was a spelling mistake in the commit message with the typo 'shorening', other than that it looks good.

          +1 to using the core function shorten_text rather than having forum_shorten_post use it's own logic.

          Show
          Mark Nelson added a comment - Hi Michael, One issue I spotted was a spelling mistake in the commit message with the typo 'shorening', other than that it looks good. +1 to using the core function shorten_text rather than having forum_shorten_post use it's own logic.
          Hide
          Mark Nelson added a comment -

          It also isn't obvious this is applicable to 2.3, 2.4 and 2.5 since there is only one diff for master. I changed the affected versions so that this is more clear.

          Show
          Mark Nelson added a comment - It also isn't obvious this is applicable to 2.3, 2.4 and 2.5 since there is only one diff for master. I changed the affected versions so that this is more clear.
          Hide
          Michael de Raadt added a comment -

          Thanks for the review, Mark.

          I've amended the Git commit message.

          Pushing to integration.

          Show
          Michael de Raadt added a comment - Thanks for the review, Mark. I've amended the Git commit message. Pushing to integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Petr Škoda added a comment -

          The correct procedure should be:
          1/ convert text to html without any filtering
          2/ shorten html text
          3/ apply filters and security cleaning on html

          Please note that it is essential to do the HTML Purifier step as the last one.

          Show
          Petr Škoda added a comment - The correct procedure should be: 1/ convert text to html without any filtering 2/ shorten html text 3/ apply filters and security cleaning on html Please note that it is essential to do the HTML Purifier step as the last one.
          Hide
          Dan Poltawski added a comment -

          Hi,

          I'm reopening this based on Petr's comments, as I think his concern is a good one (the shorten text could end up with some broken output which doing the formatting as the last step should sort out). Petr: perhaps you can provide a good example of this useage elsewhere.

          Another thing is that perhaps this would be a good opportunity to depreciate the funciton on master, as this looks like the only use in core.

          Show
          Dan Poltawski added a comment - Hi, I'm reopening this based on Petr's comments, as I think his concern is a good one (the shorten text could end up with some broken output which doing the formatting as the last step should sort out). Petr: perhaps you can provide a good example of this useage elsewhere. Another thing is that perhaps this would be a good opportunity to depreciate the funciton on master, as this looks like the only use in core.
          Hide
          Petr Škoda added a comment -

          The format_text() could probably contain a new option for text shortening which would prevent similar problems in the future, technically it would be also more correct because the filters would get the original text format and might be aware of the shortening.

          Show
          Petr Škoda added a comment - The format_text() could probably contain a new option for text shortening which would prevent similar problems in the future, technically it would be also more correct because the filters would get the original text format and might be aware of the shortening.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Michael de Raadt added a comment -

          Hi, Petr.

          That seems a bit fiddly. Does the current state of the patch reflect what you were suggesting?

          https://github.com/deraadt/moodle/compare/master...wip-mdl-23335#L1L3477

          Dan: I've added some deprecation to this. If the current patch proves to be OK, I will added lesser branches without the deprecation.

          Show
          Michael de Raadt added a comment - Hi, Petr. That seems a bit fiddly. Does the current state of the patch reflect what you were suggesting? https://github.com/deraadt/moodle/compare/master...wip-mdl-23335#L1L3477 Dan: I've added some deprecation to this. If the current patch proves to be OK, I will added lesser branches without the deprecation.
          Hide
          Petr Škoda added a comment -

          the change of $options->trusted looks like a new giant security hole to me, that is exactly the reason why the shortening should be part of format_text

          Show
          Petr Škoda added a comment - the change of $options->trusted looks like a new giant security hole to me, that is exactly the reason why the shortening should be part of format_text
          Hide
          Michael de Raadt added a comment -

          Gee you're quick.

          Actually, what I posted doesn't work. I'll put up a new patch in a minute.

          Show
          Michael de Raadt added a comment - Gee you're quick. Actually, what I posted doesn't work. I'll put up a new patch in a minute.
          Hide
          Michael de Raadt added a comment -

          It looks like a giant mess, but without changing format_text() does it achieve what you were suggesting?

          Show
          Michael de Raadt added a comment - It looks like a giant mess, but without changing format_text() does it achieve what you were suggesting?
          Hide
          Michael de Raadt added a comment -

          I've had a chat with Petr and clarified his comments.

          The latest commits include extra filtering for security.

          Show
          Michael de Raadt added a comment - I've had a chat with Petr and clarified his comments. The latest commits include extra filtering for security.
          Hide
          Michael de Raadt added a comment -

          The master branch also deprecates the forum post shortening function.

          Show
          Michael de Raadt added a comment - The master branch also deprecates the forum post shortening function.
          Hide
          Mark Nelson added a comment - - edited

          Hi Michael,

          You have actually created new repositories for your different Moodle versions, not branches, which is why the diff URL looks very different to normal and sort of confused me. So, in your case the 'Pull 2.x branch' field in the tracker is actually linking to a whole new repository! (I have updated the links accordingly) Ideally there should be one 'moodle' repository that contains all your branches which you then create comparisons between on tracker.

          Ok, to the review.

          For all versions:

          1. The logic looks sounds. I was going to ask why format_text was repeated twice but I have read Petr's comments as to why this is preferred.
          2. I might have suggested creating a separate function to perform this process but there are already so many functions in mod/forum/lib.php and the code (not yours) in general is very poor.
          3. Missing a full-stop after "// Prepare shortened version by security checking, applying shortening then applying other filters".

          For 2.4:

          1. I think you should squash your commits. Currently both commits have the same message which is not very useful at all. The second commit simply adds more comments and removes whitespace so is not necessary as a separate commit.

          For master:

          1. Woah, that's a lot of commits! Please squash this into one commit.
          2. Most debugging messages use ", please", rather than a full-stop but this is not necessary.
          3. In the PHPDocs for your function you may want to add "@see shorten_text()" for IDEs.
          4. You should record the fact that you deprecated the function somewhere in Moodle, I suggest mod/forum/upgrade.txt.

          Other than that it looks good, thanks.

          Show
          Mark Nelson added a comment - - edited Hi Michael, You have actually created new repositories for your different Moodle versions, not branches, which is why the diff URL looks very different to normal and sort of confused me. So, in your case the 'Pull 2.x branch' field in the tracker is actually linking to a whole new repository! (I have updated the links accordingly) Ideally there should be one 'moodle' repository that contains all your branches which you then create comparisons between on tracker. Ok, to the review. For all versions: The logic looks sounds. I was going to ask why format_text was repeated twice but I have read Petr's comments as to why this is preferred. I might have suggested creating a separate function to perform this process but there are already so many functions in mod/forum/lib.php and the code (not yours) in general is very poor. Missing a full-stop after "// Prepare shortened version by security checking, applying shortening then applying other filters". For 2.4: I think you should squash your commits. Currently both commits have the same message which is not very useful at all. The second commit simply adds more comments and removes whitespace so is not necessary as a separate commit. For master: Woah, that's a lot of commits! Please squash this into one commit. Most debugging messages use ", please", rather than a full-stop but this is not necessary. In the PHPDocs for your function you may want to add "@see shorten_text()" for IDEs. You should record the fact that you deprecated the function somewhere in Moodle, I suggest mod/forum/upgrade.txt. Other than that it looks good, thanks.
          Hide
          Mark Nelson added a comment -

          Actually, Michael, another thing I noticed.

          In 2.5 you are removing the line "$postcontent .= html_writer::link($discussionlink, get_string('readtherest', 'forum'));" but are not in the other branches. I suspect this deletion was an accident.

          Show
          Mark Nelson added a comment - Actually, Michael, another thing I noticed. In 2.5 you are removing the line "$postcontent .= html_writer::link($discussionlink, get_string('readtherest', 'forum'));" but are not in the other branches. I suspect this deletion was an accident.
          Hide
          Mark Nelson added a comment -

          Heh, sorry, other branches, in this case I mean other repositories.

          Show
          Mark Nelson added a comment - Heh, sorry, other branches, in this case I mean other repositories.
          Hide
          Michael de Raadt added a comment -

          Thanks for the additional feedback, Mark.

          I've fixed the various punctuation issues and niceties.

          I managed to get MSysGit to work with an editor properly to allow me to do interactive rebasing, thus permitting commit squashing.

          I've created MDL-40851 to finalise the deprecation of forum_shorten_post().

          Show
          Michael de Raadt added a comment - Thanks for the additional feedback, Mark. I've fixed the various punctuation issues and niceties. I managed to get MSysGit to work with an editor properly to allow me to do interactive rebasing, thus permitting commit squashing. I've created MDL-40851 to finalise the deprecation of forum_shorten_post().
          Hide
          Sam Hemelryk added a comment -

          Hi Michael,

          I'm sending this back this week sorry, there are a couple of things that I've noted here.

          1. There is no "filters" option - it is "filter" - https://github.com/deraadt/moodle/compare/wip-mdl-23335-master#L1R3493
          2. The deprecated forum function needs to be left in the forum library file, only core functions get moved to deprecatedlib.php. Stripping forums twisted integration with core is an ongoing task that will need to be completed before we change/upgrade our forum module and such a change would go against the nature of what is going to need to happen there.
          3. On a minor note the deprecated method states since 2.5 and there is a missing space on the first line of the phpdoc for the function.
          4. It would be really nice to prehaps get a unit test for the shorten text function to help prove it works. Certainly it would aid us in detecting any such breakages.

          Looking at it because of the mis-set of the filter option it appears that your patch works because filters are being applied on the format call that shorten_text is html aware (its much smarter than the original forum shorten function) so perhaps your master change only need to upgrade to the new shorten_text, I'll leave that you to you to work out. Worth noting is that the double format_text call was what caught me eye and the reason I left this overnight so that I could have a very good think about the consequences of such actions.

          Anyway enough for now - ping me if you've any questions

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Michael, I'm sending this back this week sorry, there are a couple of things that I've noted here. There is no "filters" option - it is "filter" - https://github.com/deraadt/moodle/compare/wip-mdl-23335-master#L1R3493 The deprecated forum function needs to be left in the forum library file, only core functions get moved to deprecatedlib.php. Stripping forums twisted integration with core is an ongoing task that will need to be completed before we change/upgrade our forum module and such a change would go against the nature of what is going to need to happen there. On a minor note the deprecated method states since 2.5 and there is a missing space on the first line of the phpdoc for the function. It would be really nice to prehaps get a unit test for the shorten text function to help prove it works. Certainly it would aid us in detecting any such breakages. Looking at it because of the mis-set of the filter option it appears that your patch works because filters are being applied on the format call that shorten_text is html aware (its much smarter than the original forum shorten function) so perhaps your master change only need to upgrade to the new shorten_text, I'll leave that you to you to work out. Worth noting is that the double format_text call was what caught me eye and the reason I left this overnight so that I could have a very good think about the consequences of such actions. Anyway enough for now - ping me if you've any questions Cheers Sam
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Michael de Raadt added a comment -

          Thanks for your feedback, Sam.

          Certainly the difference between "filter" and "filters" is significant, and it's an easy mistake to make.

          I have:

          • corrected the use of the filter option in all branches;
          • corrected the order of the statements, which would not have been shown without correcting the use of the filter option; and
          • shifted the deprecated function back to module.

          I looked at the unit tests for shorten_text() and there is already one there that involves the multi-lang filter, so I think that is covered.

          Show
          Michael de Raadt added a comment - Thanks for your feedback, Sam. Certainly the difference between "filter" and "filters" is significant, and it's an easy mistake to make. I have: corrected the use of the filter option in all branches; corrected the order of the statements, which would not have been shown without correcting the use of the filter option; and shifted the deprecated function back to module. I looked at the unit tests for shorten_text() and there is already one there that involves the multi-lang filter, so I think that is covered.
          Hide
          Damyon Wiese added a comment -

          I have been discussing this with integrators + Michael to get a better understanding of what we are trying to achieve here.

          The bit I do not like about this patch - is that it is required to call format_text, format_text, shorten_text. If this is required of all developers then surely the API is wrong - especially when we are trying to ensure the security cleaning is applied in the correct place.

          IMO: it should be safe to call simply format_text, then shorten_text and format_text should ensure filters are run if required and text is cleaned appropriately. Shorten text should handle the HTML result and sensibly shorten it without breaking any tags.
          Shorten text should be done after the filters are run to allow e.g. multilang.

          Noting that Dan has a different view that the shorten text should happen before the filtering (which would prevent multilang + shorten text combination always).

          Michael has advised to reopen so he can investigate the best approach (thanks).

          Show
          Damyon Wiese added a comment - I have been discussing this with integrators + Michael to get a better understanding of what we are trying to achieve here. The bit I do not like about this patch - is that it is required to call format_text, format_text, shorten_text. If this is required of all developers then surely the API is wrong - especially when we are trying to ensure the security cleaning is applied in the correct place. IMO: it should be safe to call simply format_text, then shorten_text and format_text should ensure filters are run if required and text is cleaned appropriately. Shorten text should handle the HTML result and sensibly shorten it without breaking any tags. Shorten text should be done after the filters are run to allow e.g. multilang. Noting that Dan has a different view that the shorten text should happen before the filtering (which would prevent multilang + shorten text combination always). Michael has advised to reopen so he can investigate the best approach (thanks).
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Michael de Raadt added a comment -

          Thanks for looking at this, Damyon. And I thought this was going to be a simple fix.

          Well, it looks like the fix is going to end up being a simple one, but the can-of-worms that it opened is not so simple. I plan to simplify the patch again and raise an issue to consider the ordering of filtering in relation to cleaning.

          Show
          Michael de Raadt added a comment - Thanks for looking at this, Damyon. And I thought this was going to be a simple fix. Well, it looks like the fix is going to end up being a simple one, but the can-of-worms that it opened is not so simple. I plan to simplify the patch again and raise an issue to consider the ordering of filtering in relation to cleaning.
          Hide
          Michael de Raadt added a comment -

          I have raised MDL-41561 as a security issue to consider the potential problems that may come about due to the ordering of cleaning, applying filters and shortening.

          Show
          Michael de Raadt added a comment - I have raised MDL-41561 as a security issue to consider the potential problems that may come about due to the ordering of cleaning, applying filters and shortening.
          Hide
          Michael de Raadt added a comment -

          I have simplified and squashed my commits.

          Show
          Michael de Raadt added a comment - I have simplified and squashed my commits.
          Hide
          Damyon Wiese added a comment -

          This matches what I think is the correct solution to this issue now, thanks Michael.

          It would be good to get another Integrator to check this to make sure we are in agreement.

          Show
          Damyon Wiese added a comment - This matches what I think is the correct solution to this issue now, thanks Michael. It would be good to get another Integrator to check this to make sure we are in agreement.
          Hide
          Michael de Raadt added a comment -

          Thanks, Damyon.

          I've put this back up for integration. I'll leave it for you guys to fight over.

          Show
          Michael de Raadt added a comment - Thanks, Damyon. I've put this back up for integration. I'll leave it for you guys to fight over.
          Hide
          Dan Poltawski added a comment -

          (Would be interested to see if Petr points out some sticky issue with doing it Damyon's suggested way)

          Show
          Dan Poltawski added a comment - (Would be interested to see if Petr points out some sticky issue with doing it Damyon's suggested way)
          Hide
          Damyon Wiese added a comment -

          Thanks Michael,

          I've integrated this to 24, 25 and master. I had to fix a white space error on the master branch.

          Cheers, Damyon

          Show
          Damyon Wiese added a comment - Thanks Michael, I've integrated this to 24, 25 and master. I had to fix a white space error on the master branch. Cheers, Damyon
          Hide
          Ankit Agarwal added a comment -

          works as described.
          Thanks

          Show
          Ankit Agarwal added a comment - works as described. Thanks
          Hide
          Damyon Wiese added a comment -

          This issue along with 77 of it's friends has been sent upstream and released to the world.

          Thankyou for your contribution.

          Show
          Damyon Wiese added a comment - This issue along with 77 of it's friends has been sent upstream and released to the world. Thankyou for your contribution.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: