error when changing ereg_replace to preg_replace

Question

I am working on old sites and updating the deprecated php functions. I have the following code, which creates an error when I change the ereg to preg.

private function stripScriptTags($string) {
    $pattern = array("'\/\*.*\*\/'si", "'<\?.*?\?>'si", "'<%.*?%>'si", "'<script[^>]*?>.*?</script>'si");
    $replace = array("", "", "", "");
    return ereg_replace($pattern, $replace, $string);
}

This is the error I get:

Fatal error: Allowed memory size of 10000000 bytes exhausted (tried to allocate 6249373 bytes) in C:\Inetpub\qcppos.com\library\vSearch.php on line 403

Is there something else in that line of code that I need to be changing along with the ereg_replace?

I understand that, but the notes I was given do not tell me what else to change. That is why I'm here. I've tried changing it to `return preg_replace($pattern, $replace, addslashes($string));` but an error comes from that too. I'm doing something wrong but I don't know what. — Jamie, Jun 19 '12 at 21:55
Hm, `'\/\*.*\*\/'si` should probably be `'\/\*.*?\*\/'si`, other then that.... — Wrikken, Jun 19 '12 at 21:56
@Wrikken can you tell me why that needs to be changed? I'm new to this conversion so I appreciate the learning aspect. — Jamie, Jun 19 '12 at 21:59
maybe he hasn't noticed your terminators, it's not common to see ' used as one, / or # are the common cases — Harald Brinkhof, Jun 19 '12 at 22:04
I _did_ notice the terminators, thankyouverymuch, I just think that that part is to strip out `/* comments*/`, in which case you want an UNgreedy match to the closing tag and NOT a greedy one....(all I did was add in the `?`). — Wrikken, Jun 19 '12 at 22:13
My comment wasn't about you, Wrikken, sorry that it rubbed you the wrong way. I thought dagon might not have noticed hence his comment. In any case its not meant to be insulting, sorry for my poor wording. — Harald Brinkhof, Jun 19 '12 at 22:53
That's the trouble with online communication, since there is no facial emotion or tone of voice to work with, things often get misunderstood for sarcasm or rudeness. :/ Off topic, but I felt the need to throw in my 2 cents. — Jamie, Jun 20 '12 at 13:07

score 1 · Accepted Answer · answered Jun 22 '12 at 14:12

1

So your regexes are as follows:

"'\/\*.*\*\/'si"
"'<\?.*?\?>'si"
"'<%.*?%>'si"
"'<script[^>]*?>.*?</script>'si"

Taking those one at a time, you are first greedily stripping out multiline comments. This is almost certainly where your memory problem is coming from, you should ungreedify that quantifier.

Next up, you are stipping out anything that looks like a PHP tag. This is done with a lazy quantifier, so I don't see any issue with it. Same goes for the ASP tag, and finally the script tag.

Leaving aside potental XSS threats left out by your regex, the main issue seems to be coming from your first regex. Try "'\/\*.*?\*\/'si" instead.

answered Jun 22 '12 at 14:12

Niet the Dark Absol

320,036
81
464
592

Oh right, @Wrikken told me to use that also. I must've gotten rid of it when commenting the lines out. – Jamie Jun 22 '12 at 14:43
That seemed to do the trick! I took the comments from the second function as well and there were no errors. I just wish this search code wasn't so broken, because now that I know my coworkers are going to change it, I feel like I wasted a lot of time trying to figure out this problem. See what happens with the search is that when you get redirected to the search results page, the search textbox duplicates, all of the results are duplicated, the results say 26 found but only lists 3 or 4 with no paging, and somehow it is searching through the php files, not the site like it should. – Jamie Jun 22 '12 at 14:46

score 0 · Answer 2 · edited Jun 20 '12 at 19:19

0

get your memory limitations value

 ini_get('memory_limit');

and check your script with memory_get_usage() and memory_get_peak_usage() (this one needs php 5.2 or higher) if this turns out to be too low then you can set it higher with:

 ini_set("memory_limit","128M"); // "8M" before PHP 5.2.0, "16M" in PHP 5.2.0, > "128M"

Just put in whatever memory you have and works for your script. Keep in mind that this limits the individual php process; to set it globally you need to adapt your php.ini file. Obviously if it needs an insane amount to run, then consider it a monkeypatch and start rewriting it for better memory footprint.

for more info look at core php.ini directives, search for Resource Limits

edited Jun 20 '12 at 19:19

Jamie

1,579
8
34
74

answered Jun 19 '12 at 21:56

Harald Brinkhof

4,375
1
22
32

I echoed the `get_memory_usage` and it shows 499504 at the top of my screen. The error shows `Allowed memory size of 10000000 bytes exhausted (tried to allocate 3124687 bytes)`. It doesn't seem like these numbers are making any sense. If it really is only 499,504 bytes, then how is it saying that it is exhausting the 10,000,000 byte limit? – Jamie Jun 20 '12 at 16:22
10MB is for your whole script, if you load a lot of data and go over that with a reg exp then it's perfectly possible to go over that. these days the default value is set at 128MB. try to set it to more and see wether it helps. (it should, that's what the error is about after all, not enough memory) it's the limit for the process, so everything you load, everything tht runs, all those functions you call.. – Harald Brinkhof Jun 20 '12 at 19:13
You're welcome, sorry I updated so much, I really just wanted to change the " to a ' but it told me I needed more than 6 characters corrected. haha, now I see that it should probably be "memory_limit" since that is the way it is worded in the second code block. I tried setting it to "300M" but it still gave that error. It must be one of those funky php errors that is actually something different on a completely separate page. I will continue digging around the site. – Jamie Jun 20 '12 at 19:21
10MB really isn't much, is this a very old PHP install? – Harald Brinkhof Jun 20 '12 at 19:22
Yes. We are in the process of relieving older servers of their duties and I am looking at old test sites to update deprecated code so the move to the newer servers won't break the sites. – Jamie Jun 20 '12 at 19:24

score 0 · Answer 3 · answered Jun 22 '12 at 13:58

0

Since allowing higher memory allocation wasn't working, the following functions were updated as follows (due to the fact that they aren't actually doing anything but causing issues):

 private function stripScriptTags($string) 
{
/* $pattern = array("'\/\*.*\*\/'si", "'<\?.*?\?>'si", "'<%.*?%>'si",
   "'<script[^>]*?>.*?</script>'si");
   $replace = array("", "", "", "");
   return ereg_replace($pattern, $replace, $string);
*/
   return $string;
}

private function clearSpaces($string, $clear_enters = true)
{
/*$pattern = ($clear_enters == true) ? ("/\s+/") : ("/[ \t]+/");
  return preg_replace($pattern, " ", trim($string));
*/
  return $string;
}

answered Jun 22 '12 at 13:58

Jamie

1,579
8
34
74

1

So... just sweep the problem under a carpet and leave it for some other poor sod of a developer to deal with? Nice... – Niet the Dark Absol Jun 22 '12 at 14:03
It wasn't my idea, another programmer came over to help me and that's what he told me to do. :/ – Jamie Jun 22 '12 at 14:36
2

From [Computer Stupidities](http://www.rinkworks.com/stupid/cs_programming.shtml): "When I was studying programming, one of my classmates was having serious troubles with his program. When he asked me for help, I leaned over his screen and saw all of his code in comments. The reason: "Well, it compiles much faster that way." " – Niet the Dark Absol Jun 22 '12 at 14:38
LOL....but I remember now, he told me to comment it out because this is all part of a search that is written badly. We are going to use a different search plugin once we find one. – Jamie Jun 22 '12 at 14:38

error when changing ereg_replace to preg_replace

3 Answers3