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.

Like anything that shouldn't be done, I decided to see if it is possible to match <script> tags robustly using regexes in PHP. Since there is no arbitrary nesting, I figured it should at least be possible.

This is what I came up with. It is designed to handle every edge case I could think of, including:

  • arbitrary attributes in the opening script tag
  • single and multiline comments and single and double-quoted strings (which might include arbitrary escape sequences) in the javascript which may contain the characters </script>
  • Captures the smallest script tag it finds.

Did I miss anything? Ideally, I want it to match exclusively anything a browser would consider a script element (might not be possible), but at the very least, I would like it to match only well-formed script tags with well-formed javascript.

Here is the string for the regex that I am passing to preg_match:

'#<script(?:[^>"]*(?:"[^"]*")?)*>((?:"(?:[^\\\\\\n"]*(?:\\\\.)*)*"|\'(?:[^\\\\\\n\']*(?:\\\\.)*)*\'|<[^/]?|/\\*(?:[^*]|\\*[^/]?)*\\*/|//.*|/[^/*]|[^\'"</])*)</script>#';

Note: I am not using this in production.

share|improve this question
3  
The <center> cannot hold. –  Chris Farmer Feb 4 at 17:34
add comment

2 Answers

up vote 10 down vote accepted

Instead of writing a long uncommented regex, compose it out of multiple parts. This makes it easier to understand what you're trying to do.

Let's go through the spec for a start tag:

  1. The first character of a start tag must be a "<" (U+003C) character.
  2. The next few characters of a start tag must be the element's tag name.
  3. If there are to be any attributes in the next step, there must first be one or more space characters.
  4. Then, the start tag may have a number of attributes, the syntax for which is described below. Attributes must be separated from each other by one or more space characters.
  5. After the attributes, or after the tag name if there are no attributes, there may be one or more space characters. (Some attributes are required to be followed by a space. See the attributes section below.)
  6. Then, if the element is one of the void elements, or if the element is a foreign element, then there may be a single "/" (U+002F) character. This character has no effect on void elements, but on foreign elements it marks the start tag as self-closing.
  7. Finally, start tags must be closed by a ">" (U+003E) character.

Expressed as a regex fragment, with insignificant whitespace:

$start_tag = "(?: [<] script (?: $ws+ (?:$attribute $ws+)* $attribute)? $ws* [>] )"

Attributes themselves are rather complex. They can either be empty, unquoted, single-quoted or double-quoted.

$attribute = "(?: $attr_name
              |   $attr_name $ws* [=] $ws* (?:[^${space_characters}\"'<>`&]+|$character_reference)+
              |   $attr_name $ws* [=] $ws* [\"] (?:[^\"&]|$character_reference)* [\"]
              |   $attr_name $ws* [=] $ws* ['] (?:[^'&]|$character_reference)* [']
              )"

Now once we fill in the appropriate values for character references and the space character recognized by HTML5 and possible attribute names, we have finally correctly matched the <script> start tag.

How does your solution hold up? I am led to believe that this part is supposed to match the start tag (I added whitespace for clarity):

<script (?: [^>"]* (?:"[^"]*")? )* >

Err, no. This fails to match single-quoted strings <script title='<You Fail>'>. This will also match some strings that do not contain valid HTML at all, like <script <<<<<<<>.

What is the point of this exercise? It's absolutely possible to correctly match HTML with the PCRE (it's not possible with a regular language, which has a specific computer science meaning. Too many people confuse the theoretical concept with a similarly named practical tool which happens to be more powerful). However, if you do want to do this, you have to follow the spec. Don't fudge it, read it. (Actually, I fudged it as well, but to a lesser degree. Do as I say, not as I do).

It is absolutely possible to write readable regexes. Compose them from multiple reusable parts. Use the /x option to include insignificant whitespace. But do not ram them onto a single line. That's just obfuscation, and you wouldn't do that in other languages.

share|improve this answer
    
Theoretical +1, will come back here when votes reload ;) –  Mat's Mug Feb 4 at 17:26
    
Good point about the spec, TBH I was unaware that single quotes were allowed. In defense of the start tag pattern, my working assumption was that the html was a priori valid. If I was to fix the quote issue, then under that assumption it seems like it should be fine. The thing is, I don't want to only capture valid markup. I want to capture anything that the browser might interpret as a script tag. –  Tim Seguine Feb 4 at 18:57
2  
@TimSeguine HTML5 was crafted in a most backwards-compatible way and specifies the handling of many cases that could occur in the wild, but were previously illegal – notice for example that backticks are forbidden in unquoted attribute values, as some browsers historically allowed them as a quote character. Following the HTML5 spec carefully is your best bet of parsing the input like a browser would. Of course many regex-based tools often assume that a tag looks like <([\w:]+)[^>]*> which breaks here, but don't compare yourself to them. –  amon Feb 4 at 19:11
    
Good advice all around. The reason I even came up with the idea to try to make a regex for this was due to some pretty terrible ones like that I saw on an old Stack Overflow post. I figured it must be possible to do better. –  Tim Seguine Feb 4 at 19:49
1  
@TimSeguine For smaller details, you can edit your question to add newer code (but do not modify existing code, as that would invalidate the answers). Usually, you would post a follow-up question. In the future, you can ask questions about “site etiquette” on Code Review Meta, or in the chat. –  amon Feb 4 at 20:03
show 2 more comments

Probably not the review/answer you're looking for, but...

Note: I am not using this in production.

Don't!

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.

-- Jamie Zawinski

From Jeff Atwood's Coding Horror blog:

I couldn't agree more. Regular expressions are like a particularly spicy hot sauce – to be used in moderation and with restraint only when appropriate. Should you try to solve every problem you encounter with a regular expression? Well, no. Then you'd be writing Perl, and I'm not sure you need those kind of headaches. If you drench your plate in hot sauce, you're going to be very, very sorry later.

If that doesn't convince you, maybe this classic Stack Overflow answer will.

And if that doesn't do it... My most sincere sympathies.

You want maintainable code. Not just code that works: code that you can read and that your successors can maintain. Just don't. do. that.

Regular Expressions are a formidable hammer. It's just unfortunate that not every problem is a nail.

Any decent HTML parser will find the <script> tags, and leave you with readable code.


If I remove the #'s from your string, I can use Expresso to analyze your regular expression:

enter image description here

That's a nasty one, but it seems to effectively work:

enter image description here

The problem with reviewing complex regular expressions like this, is that something like the "select from 7 alternatives" part, is really hard to decypher.

If you're going the way, try to use comments. Comment each alternative, say why you need each one - because at a glance, it's far from obvious!

I like that you're getting the content of the tag in a capture group though.

share|improve this answer
2  
I think this is a fair answer. Rest easy knowing that you don' t need to convince me. But I was hoping for some feedback on the regex itself. –  Tim Seguine Feb 4 at 15:15
2  
@TimSeguine added some more meat :) –  Mat's Mug Feb 4 at 15:27
1  
Which flavor of regex does Expresso support, btw? I ask because I didn't design it specially to capture html comments like the one you tested. This tells me important information, namely that there is likely to be a bug. That underscores I think the fact that the first part of your post is the important part. –  Tim Seguine Feb 4 at 16:50
    
This is not a code review, it's a rant about the unsuitability of regexes for complicated tasks. Theoretical -1. –  amon Feb 4 at 17:12
1  
Ironically I encountered a bug today in an EDI application I wrote a few months ago that parses the data files using... regex! And the bug was? In the most complicated regex pattern, of course! (look who's talking answer?) –  Mat's Mug Feb 4 at 23:56
show 2 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.