Tell me more ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 0 down vote favorite

I'm having issues with a pair of functions I have that allow a user to make links without prior html knowledge using a [link] and [/link] set. All was well until I began testing it for accidentals such as spaces before and after the link. Now it's timing out on my server which is set to a 30 second max processing time.

The user can enter a total of 5000 characters which is a healthy page worth of text that these functions have to look through. However, when the user correctly types the code in (without spaces) it does not tax the server and my page pops up in a fraction of a second like it should. If one of the [link] [/link] is missing, the while loop exits - nothing happens. It's only when the link is typed with spaces like this:

[link] www.google.com[/link] --or-- [link]www.google.com [/link]

both of which time out the server with this error (slightly modified path):

Fatal error: Maximum execution time of 30 seconds exceeded in /home/example/thiscode.php on line 18

[link]www.google.com[/link] does not time out nor does: [link]www. google.com[/link] which simply doesn't get recognized as a valid html link but does get the anchor tag put with it properly.

Any thoughts? Code hoy!

function my_link($snip)
{

    // Locate '[link]'
    $pos = stripos($snip, '[link]');

    // Strips off everything before [link]
    $str = substr($snip, $pos);

    // Removes [link] from $str
    $str = substr($str, 6);

    // Trim off any accidental whitespace BEFORE the link
    $str = ltrim($str);

    // Locate the end delimiter '[/link]'
    $pos_2 = stripos($str, '[/link]');

    // Cut off everything after and including '[/link]'
    $str = substr($str, 0, $pos_2);

    // Trim any accidental whitespace AFTER the link
    $str = rtrim($str);

    // Construct valid HTML link using content given
    if(strpos($str, 'http://') === FALSE) {
        $link = '<a href="http://'.$str.'">'.$str.'</a>';
    }else{
        $link = '<a href="'.$str.'">'.$str.'</a>';
    }

    // Replace the old [link]content[/link] with our newly constructed link
    $new_input = str_replace('[link]'.$str.'[/link]', $link, $snip);

    return $new_input;

}

function making_links($input)
{

    // Loop through $input as many times as [link] & [/link] pairs occur
    while(strpos($input, '[link]') && strpos($input, '[/link]')) {

        $input = my_link($input);

    }

    return $input;

}

Thanks in advance to anyone who can help me!

share|improve this question
I should add, the function that gets called is the second one - making_links($input) – Rohjay May 4 '11 at 4:48

3 Answers

up vote 1 down vote accepted

You could try something like this however it's likely to suffer performance issues given a suitably large block of text. Couldn't be worse than what you've got there

$input = preg_replace('@\[link\]\s*(https?://)?(.+?)\s*\[/link\]@i', '<a href="http://$2">$2</a>', $input);

For a more robust solution, you should look into lexical analysis.

share|improve this answer
I'll give it a try and let you know... Thanks! – Rohjay May 4 '11 at 4:58
Holy smokes! That works except for one issue now... I can no longer determine if I need to include 'http://' or not from what they typed in... is there a way to make that conditional with a regular expression? PS - that was just as fast as mine whether typed correctly or incorrectly! – Rohjay May 4 '11 at 5:01
1  
@Rohjay: php.net/preg_replace_callback may help you. – icktoofay May 4 '11 at 5:03
@Rohjay Updated my answer. Note, this will replace https links with http but then, your version wasn't much better ;-) – Phil May 4 '11 at 5:09
Hey Phil - thanks a ton man! I took your update and changed it so that if it found the http:// or (http?://) it removed it, then when it replaces the [link][/link] pair with the link it adds it in there - works like a charm! Man I gotta look into regular expressions! – Rohjay May 4 '11 at 5:17
up vote 1 down vote

You have a few logical errors in your code. For example, all your code up until (but not including) str_replace allows these variations:

  • [LINK]http://...[/LINK]
  • [link] www.example.com [/link]
  • [LiNk]example.com[/LINk]

But your str_replace do not allow different tag casing, and it does not allow the leading/ending spaces since those are stripped away from $str. In all the above cases, str_replace won't replace anything. In the second of these examples your code will be stuck in an infinite loop.

Use a regular expression as shown by Phil. It's by far the easiest solution to your problem.

share|improve this answer
+1 Thanks Emil for the look and suggestion - I gave his suggestion a try and it worked. I'll have a look at what you're saying here - I need to get better at coding and this is exactly what I need! Thanks! – Rohjay May 4 '11 at 5:20
up vote 1 down vote

The PECL BBCode might be an option for you. Barring that, the problem with your code is obviously with the loop. Specifically, either [link] or [/link] is not being removed from the string if there is a leading or trailing space.

Your specific issue is being caused by this line: $new_input = str_replace('[link]'.$str.'[/link]', $link, $snip);

Because you are updating the string to remove the spaces, [link]'.$str.'[/link] does not exist in the original snippet, only the form with the spaces exists. Thus, it is never found. To get around this it would be better to do work on the original string.

Another suggestion: Change strpos($input, '[link]') to strpos($input, '[link]') === FALSE. Otherwise, it will still evaluate to false if the input starts with [link]

share|improve this answer
+1 - thanks for the help tandu! I've read and reread what you're saying. I'm still relatively new to this so I'm going to copy paste your answer here and go through it alone now... Much appreciated for the help! – Rohjay May 4 '11 at 6:58

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.