Does not work with non-English Drupal installation [#1679778]

I'd like to thank the effort of making this module available for D7 first.

The following code in the module makes role names with non-ASCII character to be stripped off, entirely in some cases.

$perm .= preg_replace('/[^a-zA-Z0-9]/', '', $role_name);

The regular expression totally replaces Chinese/Japanese characters with nothing, for example. The role name ended up with an empty string, and making the module unfunctional.

A quick patch is not to replace any character, but use the role name as is. I don't know why the role name can be used as is...

I noticed some other minor I18N related issues (I don't know they warrant a bug report by themselves).

The module is somehow reported to have a non UTF-8 character. I found N+~ character (used in Spanish, I guess), which is not saved as UTF-8.
The permission screen cannot be translated due to lack of t() function. E.g. 'Edit users with no custom roles'

Comment	File	Size	Author
#5	administerusersbyrole-transliterate-5.patch	835 bytes	Niremizov
#5
#3	administerusersbyrole-transliterate-0.patch	825 bytes	smokris
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

v1adimir CreditAttribution: v1adimir commented 2 September 2012 at 15:48

it may be dangerous just removing
$perm .= preg_replace('/[^a-zA-Z0-9]/', '', $role_name);

We can use transliteration module API to generate latin name for hook_permission()

Third party developers seeking an easy way to transliterate text or file names
may use transliteration functions as follows:
if (function_exists('transliteration_get')) {
  $transliterated = transliteration_get($text, $unknown, $source_langcode);
}

Comment #2

NobuT CreditAttribution: NobuT commented 7 September 2012 at 14:30

it may be dangerous just removing

Just curious, why is it dangerous? Is there any possibility of injecting JavaScript or something to the role name? The regular expression also prohibit the use of some letters with acute or umlaut. I just wonder what was the intention of the regular expression...

Comment #3

smokris

Athens, Ohio, USA

CreditAttribution: smokris commented 16 September 2012 at 18:02

Version:	7.x-1.0-beta1	» 7.x-1.x-dev
Status:	Active	» Needs review

File	Size
administerusersbyrole-transliterate-0.patch	825 bytes

It's dangerous because some characters are not allowed in permissions strings, and invalid characters can result in database corruption.

I added the preg_replace call to address #709068: Square brackets [] in role name. There are a bunch of characters that aren't permitted to be used in permissions strings, but *which* characters aren't permitted unfortunately doesn't seem to be documented anywhere. In addition to square brackets mentioned in that issue, this comment says that commas are invalid.

We can use transliteration module API to generate latin name for hook_permission()

That sounds like a reasonable solution. If transliteration.module is installed, we should transliterate to Latin, then apply the existing preg_replace. Patch attached.

Comment #4

NobuT CreditAttribution: NobuT commented 17 September 2012 at 11:34

Thank you, Steve. I didn't know there are characters that aren't permitted in permission strings, while they are accepted as a role name. Very interesting.

Comment #5

Niremizov CreditAttribution: Niremizov commented 21 March 2013 at 15:31

File	Size
administerusersbyrole-transliterate-5.patch	835 bytes

Thx for the patch @smokris. I have applied it, but unfortunatelly it does not work as desired. For some reason you have transliterated $perm not $role_name variable. So I rerolled your patch, see below.

Comment #6

AdamPS CreditAttribution: AdamPS commented 21 November 2014 at 15:00

Assigned:	Unassigned	» AdamPS
Issue summary:	View changes

I intend to fix this as part of #2378869: Meta-issue for Beta 2 release. Please sign up as a follower of that issue and there will shortly be a patch that I would like feedback on.

The fix is to use role ID rather than role name to build the permission. We still use the name for the title that is visible to users.

Thanks for the patch, but I think role ID approach is better and also fixes several related issues, e.g. renaming roles.