EZP-26276: Exclamation mark followed by non-chars at end of URL does not work in emails

[gabrielfin]

Fix for https://jira.ez.no/browse/EZP-14719 (old zombie issue)
https://jira.ez.no/browse/EZP-26276 (new incarnation)

This is "My Article!" becomes This-is-My-Article!. The exclamation mark should not be there.

[yannickroger]

ping @glye it looks like this one came back from the dead to eat your brain 😄

[glye]

@gabrielfin Thanks. Which version(s) does this affect?

Note that we need a different approach to make this complete. The PR as is fixes the case of This is "My Article!" but just to be difficult, let's try these ;)
This is "My Article!"!" -> This-is-My-Article!
This is "My Article!"!"!" -> This-is-My-Article!-!
So we need either smarter regexps here, or a loop.

And this fix in commandUrlCleanup should probably be duplicated in commandUrlCleanupIRI below.

[glye]

FYI I made a new issue, and updated the title/description.

[gabrielfin]

This issue affects, at least, master and 4.2.0. But I guess it actually affects every legacy version since the original patch (EZP-14719).

How about if "Remove dots at beginning/end" actually strips them instead of replacing them with $sep, and we add {$sepQ} to the second bracket expression. ie:

        $text = preg_replace( array( "#[^a-zA-Z0-9_!\.-]+#",
                                     "#^[\.]+|[!\.{$sepQ}]+$#", # Remove dots at beginning/end
                                     "#\.\.+#", # Remove double dots
                                     "#[{$sepQ}]+#", # Turn multiple separators into one
                                     "#^[{$sepQ}]+|[{$sepQ}]+$#" ), # Strip separator from beginning/end
                              array( $sep,
                                     "",
                                     $sep,
                                     $sep,
                                     "" ),
                              $text );

Perhaps this new regex could also be merged with the last regex.

[glye]

@gabrielfin That sounds ok to me.

[gabrielfin]

I updated the PR with the proposed regex. I also merged it with the last regex, since they were actually very similar and it also fixes an additional test case: dot preceded by a non char at the beginning of the url (ie ".My Article" turned (wrongly) into .My-Article)

I also applied the same to commandUrlCleanupIRI.

On a sidenote: the usage of $prepost seems unnecessary. It strips trailing spaces, which were already stripped, and it strips trailing dashes when dash is not the separator (why?). However, I just left it as is.

[glye]

+1, works in my test.
@yannickroger, you want to have a look again? :)

[yannickroger]

I had a look, it's full of regular expressions 😄
but +1

[glye]

Note: This does not strip exclamation marks from the beginning of strings, but the original didn't do that either.

[glye]

Merged. Thanks @gabrielfin!