Simplify the code for sanitize class #825

Merged
merged 1 commit into from Sep 13, 2012
View
102 lib/Cake/Utility/Sanitize.php
@@ -46,14 +46,15 @@ public static function paranoid($string, $allowed = array()) {
}
}
- if (is_array($string)) {
- $cleaned = array();
- foreach ($string as $key => $clean) {
- $cleaned[$key] = preg_replace("/[^{$allow}a-zA-Z0-9]/", '', $clean);
- }
- } else {
- $cleaned = preg_replace("/[^{$allow}a-zA-Z0-9]/", '', $string);
+ if (!is_array($string)) {
+ return preg_replace("/[^{$allow}a-zA-Z0-9]/", '', $string);
+ }
+
+ $cleaned = array();
+ foreach ($string as $key => $clean) {
+ $cleaned[$key] = preg_replace("/[^{$allow}a-zA-Z0-9]/", '', $clean);
}
+
return $cleaned;
}
@@ -70,14 +71,12 @@ public static function escape($string, $connection = 'default') {
return $string;
}
$string = $db->value($string, 'string');
- if ($string[0] === 'N') {
- $string = substr($string, 2);
- } else {
- $string = substr($string, 1);
+ $start = 1;
+ if ($string{0} === 'N') {
+ $start = 2;
}
- $string = substr($string, 0, -1);
- return $string;
+ return substr(substr($string, 1), 0, -1);
}
/**
@@ -128,8 +127,7 @@ public static function html($string, $options = array()) {
* @return string whitespace sanitized string
*/
public static function stripWhitespace($str) {
- $r = preg_replace('/[\n\r\t]+/', '', $str);
- return preg_replace('/\s{2,}/u', ' ', $r);
+ return preg_replace('/\s{2,}/u', ' ', preg_replace('/[\n\r\t]+/', '', $str));
}
/**
@@ -139,10 +137,13 @@ public static function stripWhitespace($str) {
* @return string Sting with images stripped.
*/
public static function stripImages($str) {
- $str = preg_replace('/(<a[^>]*>)(<img[^>]+alt=")([^"]*)("[^>]*>)(<\/a>)/i', '$1$3$5<br />', $str);
- $str = preg_replace('/(<img[^>]+alt=")([^"]*)("[^>]*>)/i', '$2<br />', $str);
- $str = preg_replace('/<img[^>]*>/i', '', $str);
- return $str;
+ $preg = array(
+ '/(<a[^>]*>)(<img[^>]+alt=")([^"]*)("[^>]*>)(<\/a>)/i' => '$1$3$5<br />',
+ '/(<img[^>]+alt=")([^"]*)("[^>]*>)/i' => '$2<br />',
+ '/<img[^>]*>/i' => ''
+ );
+
+ return preg_replace(array_keys($preg), array_values($preg), $str);
}
/**
@@ -152,7 +153,8 @@ public static function stripImages($str) {
* @return string String with <script>, <style>, <link>, <img> elements removed.
*/
public static function stripScripts($str) {
- return preg_replace('/(<link[^>]+rel="[^"]*stylesheet"[^>]*>|<img[^>]*>|style="[^"]*")|<script[^>]*>.*?<\/script>|<style[^>]*>.*?<\/style>|<!--.*?-->/is', '', $str);
+ $regex = '/(<link[^>]+rel="[^"]*stylesheet"[^>]*>|<img[^>]*>|style="[^"]*")|<script[^>]*>.*?<\/script>|<style[^>]*>.*?<\/style>|<!--.*?-->/is';
@ADmad
CakePHP member
ADmad added a line comment Sep 12, 2012

Isn't this change contrary to the other changes? You are adding an assignment instead of removing. The line remains too long regardless.

@dogmatic69
dogmatic69 added a line comment Sep 12, 2012

It is slighty, but fits in more with keeping the code more or less on the screen.

As it was you could not see most of it. It can be undone if its better on one long line.

@bmcclure
bmcclure added a line comment Sep 12, 2012

I agree with this change.

I'm fine with the longer line since it's just a string. It's still much clearer since you don't have function parameters hanging off the edge of the screen.

Shortening the line would be great, but I'm not sure. Would chunking that regex out into more manageable pieces make things easier, or more confusing?

@markstory
CakePHP member
markstory added a line comment Sep 13, 2012

I find regex's in bits hard to understand as I can't 'talk them out' while reading them. I'm fine with the change as it stands.

@dogmatic69
dogmatic69 added a line comment Sep 13, 2012

How about this #831

Seems cleaner like that to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return preg_replace($regex, '', $str);
}
/**
@@ -162,10 +164,11 @@ public static function stripScripts($str) {
* @return string sanitized string
*/
public static function stripAll($str) {
- $str = Sanitize::stripWhitespace($str);
- $str = Sanitize::stripImages($str);
- $str = Sanitize::stripScripts($str);
- return $str;
+ return Sanitize::stripScripts(
+ Sanitize::stripImages(
+ Sanitize::stripWhitespace($str)
+ )
+ );
}
/**
@@ -212,10 +215,8 @@ public static function clean($data, $options = array()) {
return $data;
}
- if (is_string($options)) {
+ if (!is_array($options)) {
$options = array('connection' => $options);
- } elseif (!is_array($options)) {
- $options = array();
}
$options = array_merge(array(
@@ -235,30 +236,29 @@ public static function clean($data, $options = array()) {
$data[$key] = Sanitize::clean($val, $options);
}
return $data;
- } else {
- if ($options['odd_spaces']) {
- $data = str_replace(chr(0xCA), '', $data);
- }
- if ($options['encode']) {
- $data = Sanitize::html($data, array('remove' => $options['remove_html']));
- }
- if ($options['dollar']) {
- $data = str_replace("\\\$", "$", $data);
- }
- if ($options['carriage']) {
- $data = str_replace("\r", "", $data);
- }
- if ($options['unicode']) {
- $data = preg_replace("/&amp;#([0-9]+);/s", "&#\\1;", $data);
- }
- if ($options['escape']) {
- $data = Sanitize::escape($data, $options['connection']);
- }
- if ($options['backslash']) {
- $data = preg_replace("/\\\(?!&amp;#|\?#)/", "\\", $data);
- }
- return $data;
}
- }
+ if ($options['odd_spaces']) {
+ $data = str_replace(chr(0xCA), '', $data);
+ }
+ if ($options['encode']) {
+ $data = Sanitize::html($data, array('remove' => $options['remove_html']));
+ }
+ if ($options['dollar']) {
+ $data = str_replace("\\\$", "$", $data);
+ }
+ if ($options['carriage']) {
+ $data = str_replace("\r", "", $data);
+ }
+ if ($options['unicode']) {
+ $data = preg_replace("/&amp;#([0-9]+);/s", "&#\\1;", $data);
+ }
+ if ($options['escape']) {
+ $data = Sanitize::escape($data, $options['connection']);
+ }
+ if ($options['backslash']) {
+ $data = preg_replace("/\\\(?!&amp;#|\?#)/", "\\", $data);
+ }
+ return $data;
+ }
}