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

Already on GitHub? Sign in to your account

Simplify the code for sanitize class #825

Merged
merged 1 commit into from Sep 13, 2012

Conversation

Projects
None yet
4 participants
Contributor

dogmatic69 commented Sep 12, 2012

removing else statements and variables that are not needed.
eg: return something(); vs $foo = something(); return $foo;

@markstory markstory commented on an outdated diff Sep 12, 2012

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)) {
@markstory

markstory Sep 12, 2012

Owner

Should be if (

Owner

markstory commented Sep 12, 2012

Outside of a minor code style issue, this looks good.

@ADmad ADmad commented on the diff Sep 12, 2012

lib/Cake/Utility/Sanitize.php
@@ -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

ADmad Sep 12, 2012

Member

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 Sep 12, 2012

Contributor

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 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

markstory Sep 13, 2012

Owner

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 Sep 13, 2012

Contributor

How about this #831

Seems cleaner like that to me.

@dogmatic69 dogmatic69 Simplify the code for sanitize class
removing else statements and variables that are not needed.
eg: return something(); vs $foo = something(); return $foo;
44f8f84
Contributor

dogmatic69 commented Sep 12, 2012

I updated the if() issue on this.

@markstory markstory added a commit that referenced this pull request Sep 13, 2012

@markstory markstory Merge pull request #825 from dogmatic69/sanitize-simplify-code
Simplify the code for sanitize class
5d8ca79

@markstory markstory merged commit 5d8ca79 into cakephp:2.3 Sep 13, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment