Please upgrade here. These earlier versions are no longer being updated and have security issues.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Mentioning users with spaces in their names

judgejjudgej
edited March 2011 in Vanilla 2.0 - 2.8
Did the question of how to do this ever get resolved? I've not got 2.0.17.8 installed (really need to wait for the categories to settle down and be more stable first), so I can't test that, but I know there was discussion about how it would work.

2.0.13 Does not support sending out emails to users with spaces in their names. It makes sense to me to allow a plus (+) to be used to join the names together, e.g. @Fred+Bloggs <--you can see the problem here already

Any ideas or does anyone know whether this now works in 2.0.17?

-- Jason

Comments

  • LincLinc Detroit Admin
    Pretty sure there's no difference.
  • @judgej - you can add replacement of spaces to underscores during user registration. I think that it is good alternative.
  • Since the usernames are used as display names, I would like to keep the spaces in this case as it is linked to a legacy CMS through ProxyConnect. It displays well, looks good, and still works when sending messages to users. The profile pages for these users have a plus (+) as a space (old-style URL encoding) and that is why it thought it would be a good idea to use the same character for mentioning users.

    I bet its only a couple of lines of code to do, and I'll post back here if I work out how.
  • avantime4mikeavantime4mike ✭✭
    edited March 2011
    Any ideas or does anyone know whether this now works in 2.0.17?
    I can confirm it's exactly the same in 2.0.17
  • Thanks. I'll look into a local fix and post it here if I manage it.
  • judgejjudgej
    edited March 2011
    It is actually relatively simple to support this feature, to a certain extent at least.

    The library/core/functions.general.php function GetMentions() contains the regular expression that finds usernames being mentioned in a post. I just replaced the bit that searches for a string of word character (\w) to a match that includes plus character ([\w\+]).

    In that same function, any matching mentions that are to be returned, need the + converted back into a space. Everything after that then just works as normal. This function can be overridden in a plugin to avoid having to mess with the core.

    Doing this has also highlighted several things to me. Firstly 2.0.13 does not handle mentions correctly (duplicate emails in some instances, and the mentions not being checked at all in some comments). The expression that is used to match a mention in a comment or discussion display is not changed, so this RE is duplicated elsewhere - both should really use the same bit of code, so that both functions can be overridden by a plugin in one go.

    Lastly, this RE assumes usernames are between 3 and 20 characters (I assume it is international characters and not bytes!) long. his is an unnecessary and arbitrary limit. The username length should be defined in one central place IMO, and all references should go back there, including the creation of the username database column. I've increased mine to 40 characters already, because 20 was too short, and now I realise I need to search for other places where "20" is hard-coded in.
  • It also occurred to me, while in that code, that the ActivityModel, that logs things that happen, stores a URL for the discussion or comment against that user, but not the comment or discussion ID. That explains to me why the activity log is never updated when comments and discussions are deleted - all those URLs point to dead pages, but there are no columns in that log that can be used to determine which of those corresponding log entries can be deleted when those destination pages are removed.
  • The library function libary/core/class.format.php Gdn_Format::Mentions() uses the same regular expression to format mentioned users in discussion posts. This one cannot be overridden by a plugin, so it either needs to be fixed or hacked in the core.

    I've changed that, and now I'm happy :-)
  • Added to http://www.vanilla-wiki.info/bugs/index.php?project=1&do=index, as I also need this functionality. You can add details if you wish.
  • judgejjudgej
    edited March 2011
    I've not got a login for the bug tracker (and just about reached my limit for yet another login ;-) so here are the code changes I've made.

    Added to conf/bootstrap.before.php

    if (!function_exists('ValidateUsername')) { function ValidateUsername($Value, $Field = '') { return ValidateRegex( $Value, '/^([\d\w_\+]{3,40})?$/si' // Include "+" and extend the username length ); } } if (!function_exists('GetMentions')) { function GetMentions_ConvertPlus(&$Value, $Key) { $Value = str_replace('+', ' ', $Value); } function GetMentions($String) { $Mentions = array(); // This one grabs mentions that start at the beginning of $String preg_match_all( '/(?:^|[\s,\.])@([\w\+]{3,40})\b/i', // Include "+" and extend the username length $String, $Matches ); if (count($Matches) > 1) { array_walk($Matches[1], 'GetMentions_ConvertPlus'); $Result = array_unique($Matches[1]); return $Result; } return array(); } }

    Note the "40" above extends the length of the username to 40 chars (or bytes) and this also validates usernames with spaces in. Normally I use ProxyConnect, which bypasses this validation function, so my users have spaces in without this change. The database GDN_User.Name column also needs extending to 40 chars.

    IMO the "40" (or "20" in the standard install) needs to be a central configuration item and not hard-coded in so many places.

    A core change that cannot be added to the bootstrap is library/core/class.format.php Gdn.Format::mentions()

    ... // Handle @mentions. $Mixed = preg_replace( '/(^|[\s,\.])@([\w\+]{3,40})\b/i', //{1,20} // JDJ Include "+" as a valid character Anchor('\1@\2', '/profile/\\2'), $Mixed ); ...

    (Edit: just to show how blunt a tool the above is, the "@mentions" is being converted to a link to a non-existent profile page. It is in a code block for a start and demonstrates how poor regular expressions are at replacing text in structured documents - they just have no concept of context.)

    This could possibly be made user to upgrade by using a "replace" hook for this function in a plugin (renaming the Mentions() method to xMentions() - I've not tried this though, and I have a feeling it may only work on models and controllers).

    This is Vanilla 2.0.13, and that is where I will be stuck for a while, but Vanilla does seem to be changing fast.

    I hope that helps.
  • I added your tips here, so others could find them:

    http://www.vanilla-wiki.info/Bugs/VariousTips
  • judgejjudgej
    edited March 2011
    Thanks. Thinking about it, the user validation should not allow pluses - it should allow spaces:

    if (!function_exists('ValidateUsername')) { function ValidateUsername($Value, $Field = '') { if (preg_match('/(^[ ]|[ ]{2}|[ ]$)/', $Value)) return FALSE; // JDJ Fail on leading, trailing or runs of spaces return ValidateRegex( $Value, '/^([\d\w_ ]{3,40})?$/si' // JDJ Allow spaces and 40 char length ); } }

    That is also assuming the username will get trimmed on input, so users cannot use spaces as prefixes or suffixes. In addition I would also disallow runs of more than one space, which could probably also be detected with the RE (pop-quiz exercise for a lazy Sunday!) but would be far easier and less prone to error just to use a string-search for two spaces.

    Edit: I've added another *untested* preg_match() to fail validation on fun being had with spaces.
  • I just ported over a few thousand usernames (custom bit of sql injection) from a retiring board and the simplest solution to this problem is to *not use spaces in usernames*. Stripped them all out, along with non alpha-numerics. Problem solved. As users log-in with associated email addresses, changing their usernames doesn't break anything.
  • judgejjudgej
    edited March 2011
    @jrapage Since the usernames (used to log in, when not using emails) are the same as the display names (how people are seen), I am curious how you managed to get the display names to appear with spaces in, which is something our users want.

    "Problem solved" is always easy for a slightly different problem, and with different needs to start with ;-)
  • @judej We removed spaces, so they don't appear in display names either. It solves the space issue by avoiding the problem completely.

    If you look at the username requirements in the latest version, you'll notice spaces are not allowed for new member sign-ups.

    Sure, there are work-arounds with string manipulation when padding for @ mentions, but you may be causing yourself a legacy issue, especially as newer versions are released. Will your code patch work every time?

    Twitter, where the @mention started, don't allow spaces in usernames for good reason.

  • I dont have conf/bootstrap.before.php

    Its part of a plugin? Thanks

Sign In or Register to comment.