Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hard-coded English text strings should be moved to their language files #114

Merged
merged 1 commit into from
Jan 4, 2014

Conversation

feeva
Copy link

@feeva feeva commented Jan 4, 2014

I found most of view files and controllers were not localization-ready. So here's my little contribution to move hard-coded literal strings to language files. I think I've done only 10 percent of localization but I think I need to discuss my localization with other contributors.

  • I created form.php language files to localize creation and edit forms.
  • I think there are two many language files. Can't we just merge almost all table.phps and form.phps?
  • But before we move on, there're some inconsistencies to be resolved. For example, I found these page titles: "Create Asset" vs "License Create", "Order Number" vs "Order No."
  • Some models have validation rules like "alpha_space". I think this sould be removed or at least be replaced with regexp for non-English users. But I didn't work on this before I hear from you.

If you like my localization I'll continue to work more.

@snipe snipe merged commit 40d6ad0 into snipe:develop Jan 4, 2014
@snipe
Copy link
Owner

snipe commented Jan 4, 2014

Great stuff, thanks! The localization files being overly fragmented is a remnant from starting with the starter site I started with. It's been on my list of things to fix and tighten up (like the frontend/backend business, since there is no real front end for this app), but hadn't gotten around to it.

I'm not sure what you mean about alpha_space. alpha space comes with Laravel as a provided filter. I believe I created alpha_dash, which is more lenient and allows more special characters (but doesn't allow characters that could be used for XSS attacks.)

@feeva
Copy link
Author

feeva commented Jan 5, 2014

As for "alpha_space", I found these model classes:

  • Statuslabel
  • Model
  • Asset

maybe more.

@snipe
Copy link
Owner

snipe commented Jan 5, 2014

Yeah, I know where they are, I'm just not sure what you mean by "I think this sould be removed or at least be replaced with regexp for non-English users."

If we were going to replace validation, that's fine, but we can't just remove it. Validation exists for a reason. If you'd like to take a stab at creating a new filter that is more international-friendly, that's cool, as long as the rest of the general validation rules apply (i.e. alpha contains only letters, not spaces). What did you have in mind specifically? Allowing accented characters to start? I think it will get hairier in a whitelisted context if we start looking at kanji, etc, but open to discussion about it. I'm not sure how regex ranges work with kanji or if they do at all.

Validator::extend('alpha_space', function($attribute,$value,$parameters)
{
return preg_match("/^[-+:?#~'/()_,!. a-zA-Z0-9]+$/",$value);
});

@feeva
Copy link
Author

feeva commented Jan 5, 2014

I understand that you want to be sure XSS vunlerable characters not be accepted. But what about model fields that don't have any validation at all, e.g. order number in Asset class. And one more; model_id field is just a select field and there's no character-level validation. So you can inject any dangerous characters by forging that field values if you really want to.

I think the best we can do are 1. escape or encode any html-allowed fields in views when outputing, 2. filters are just data format filters, you should not rely on those for security of your application.

So, for no. 1, Laravel supports this:

Of course, all user supplied data should be escaped or purified. To escape the output, you may use the triple curly brace syntax:

Hello, {{{ $name }}}.

For no. 2, I think alpha_space or alpha_XXX are too limited for international users. Maybe we can have this RegExp as an alpha_space anternative (unicode space and letters).

/[\s\pL]/

or considering unicode "separator" characters

/[\Z\pL]/

@stevecookms stevecookms mentioned this pull request Jul 19, 2016
@dooley1023 dooley1023 mentioned this pull request Feb 2, 2017
2 tasks
@katecruser katecruser mentioned this pull request Feb 8, 2017
2 tasks
@Mike1704 Mike1704 mentioned this pull request Mar 3, 2017
@aaronlott aaronlott mentioned this pull request Jun 13, 2017
@hairysamurai hairysamurai mentioned this pull request Sep 12, 2017
@rkayutkin rkayutkin mentioned this pull request Jun 14, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants