current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

My users may need to supply a 'disease category' to my site. I need to let them use all alphanumeric characters, hyphens and single quotes. Would someone review this to see if they feel it's reasonable safe? I'm very happy for suggestions to make it tighter,

      $disease_category = "some user input";
      $reg = "#[^'a-z0-9\s-]#i";
      preg_match($reg,$disease_category);
share|improve this question
    
I don't think you've asked a good, clear question. The code doesn't really make sense ($disease_category vs. $new_category, the unexplained \s in the regex, and the discarding of the result of preg_match()). Also, "safe" is nebulous term, since it all depends on what you intend to do with the string. –  200_success♦ Jun 23 at 5:50
    
I have edited the $new_caetgory which should be $disease_category. I disagree with the clarity bit though - I think the question is very clear - I want the preg_match to match all alphanumeric characters, hyphens and single quotes. Nothing else. As I am new to regex, and this is really my first attempt to construct the correct pattern, I wanted to get some expert feed back on whether my pattern serves the purpose required. –  GhostRider Jun 23 at 10:58
1  
Many thanks to Madara and 200_success who have schooled me in the difference between validation and security. –  GhostRider Jun 23 at 13:16
add comment

2 Answers

up vote 2 down vote accepted

Well if having an underscore in your line could be acceptable, then

$reg = "#[^\w\s-']#i";

could be used. If you cannot have underscores, then

$reg = "#[^a-z\d\s-']#i";

could be correct.

A conditional using preg_match would be safe with this.

share|improve this answer
    
Got it. Many thanks –  GhostRider Jun 20 at 19:55
2  
The - should be the first, or last part of a regex character class section [...]. It otherwise needs to be escaped, or it becomes a range indicator. –  rolfl♦ Jun 21 at 14:07
    
Update ... actually, this will work, but in general, the meaning of the - in a regex is complicated, and the internal use of - as a non-range character is ... unexpected, for me at least. –  rolfl♦ Jun 21 at 14:18
    
@rolfl Regex101.com comes in handy! I hadn't thought of the confusion you pointed out, in future cases I'll make sure order them in a more clear way –  Alex L Jun 21 at 20:48
    
Why not start now? :) –  Jack Jun 23 at 11:28
add comment
up vote -4 down vote

Found this: http://stackoverflow.com/questions/8013897/accept-international-name-characters-in-regex so this is actually a duplicate.

\p{L} should work? Depends on language though. I normally use the actual characters though, so you can set a range like this if it's utf-8: À-ÿ you must be in unicode mode to do the \p{L} version I think. (PHP)

$reg = "#[^a-zA-ZÀ-ÿ\d\s-']#i";

Please mark as answered XD

share|improve this answer
1  
The capitals are covered because the regex has the 'i' flag set, and, the specification appears to be that the exact characters are well defined, adding in modifiers and so on appears to be out of scope. –  rolfl♦ Jun 21 at 14:10
    
It says, "all alphanumeric characters", and I don't know where OP is from. –  Darius Jun 21 at 14:13
    
He's from London. Lots of languages spoken there ;) –  Darius Jun 21 at 14:15
    
You got me looking...., it appears that despite your assertion, Alphanumeric - is used to describe the collection of Latin letters and Arabic digits or a text constructed from this collection. –  rolfl♦ Jun 21 at 14:24
    
LOL! Alpha - alphabet, Numeric - numbers. Potential for debate here. Alphabetic is relative unless you specify a standard. I don't think you can tell a Russian that "для" is not alphabetic. This is a bit silly tho. Cheerio ;) whatis.techtarget.com/definition/alphanumeric-alphameric Wikipedia is just Wikipedia. –  Darius Jun 21 at 14:44
show 4 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.