URL Regex is not working

Using Perl, I am trying to parse a bunch of XML fi开发者_StackOverflowles and trying to find any form of URL in the XML and print it. My regex does not seem to work and it is not returning any match. What am i missing?

sub findURL{
local($inputLine, $outText);
$inputLine = $_[1];
 while (length($inputLine) > 0)
 {
 if ($inputLine =~ /^(((http|https|ftp):\/\/)?([[a-zA-Z0-9]\-\.])+(\.)([[a-zA-Z0-9]]){2,4}([[a-zA-Z0-9]\/+=%&_\.~?\-]*))*$/ )

 {
 $outText .= $&;
 $inputLine = $';
 }
 else
 {
  $inputLine = "";
  $outText .= "";
 }
 }
 return $outText;
}

---

use Regexp::Common

use Regexp::Common qw /URI/;

while (<>) {
    /$RE{URI}{HTTP}/       and  print "Contains an HTTP URI.\n";
}

---

Your code is seven different shades of wrong:

• You shouldn't use a regex to parse XML (see this question)
• local should probably not be used that way, you probably want my
• The $&, $', and $` variables should not be used (use captures instead)
• Your indenting is terrible
• $inputLine = $_[1]; grabs the second argument to the function (what is the first?)
• if you are going to use a regex, you should use the /g regex modifer, not roll your own multiple match code
• your regex is capturing stuff it shouldn't (use (?:) for grouping, not ())

Here is how I would write your code if I didn't care that I would grab stuff I shouldn't and might miss stuff that I want (because a regex can't be smart enough to parse the XML). Note how the URL in the comment gets grabbed.

#!/usr/bin/perl

use strict;
use warnings;

use Regexp::Common qw/URI/;

sub find_urls {
    my $text = shift;
    return $text =~ /$RE{URI}{-keep}/g;
}

my $xml = do { local $/; <DATA> };

for my $url (find_urls($xml)) {
    print "$url\n";
}

__DATA__
<root>
    this is some text
    and a URL: http://foo.com/foo.html
    this isn't a URL http:notgrabbed.com
    <img src="http://example.com/img.jpg" />
    <!-- oops, shouldn't grab this one: ftp://bar.com/donotgrab -->
</root>

---

Use the URI::Find and URI::Find::Schemeless modules, available from the CPAN. For example

#! /usr/bin/perl

use warnings;
use strict;

use URI::Find;
use URI::Find::Schemeless;

my $xml = join "" => <DATA>;
URI::Find            ->new(sub { print "$_[1]\n" })->find(\$xml);
URI::Find::Schemeless->new(sub { print "$_[1]\n" })->find(\$xml);

__DATA__
<foo>
  <bar>http://stackoverflow.com/</bar>
  <baz>www.perl.com</baz>
</foo>

Output:

http://stackoverflow.com/
www.perl.com

---

I think it's what you think is a character class. For some reason that compiles, but the debug output shows something curious when I isolated the character class.

use strict;
use warnings;
use re 'debug';

my $re = qr/[[a-zA-Z0-9]\-\.]/;

And the debut output (from use re 'debug') shows this:

Compiling REx "[[a-zA-Z0-9]\-\.]"
Final program:
   1: ANYOF[0-9A-[a-z][] (12)
  12: EXACT <-.]> (14)
  14: END (0)
anchored "-.]" at 1 (checking anchored) stclass ANYOF[0-9A-[a-z][] minlen 4 

So it's looking for the literal string '-.]' as an "anchor". Thus if your hostname does not have '.-]' in it will never match. Thus it is like I said before, you're closing your character class with the first non-escaped ']'.

The best way to include a dash is to make it the last character of the class--so as to remove the possibility that it can indicate a range.

In addition, it should all just be one class. You actually close the class with the first non-escaped square-bracket close. Your character class should read:

[a-zA-Z0-9.-]

And that's all.

In addition, it's probably better practice to use named character classes as well:

[\p{IsAlnum}.-]

• Another interesting thing I found out is that in ']' is interpreted as a literal square-close wherever a character class is not open. Thus you only need to escape it to avoid ending a character class, and thus, included it. Conversely '[[' will include '[' into the character class, thus there is no reason to escape a '[' unless outside of a character class.

---

A few comments not directly related to your question, but to your code.

1. I don't understand why you use local in the context you provided. My gut feeling is that you should use my instead of local.
2. $inputLine = $_[1] actually means that you want to assign the second argument you pass to findURL to $inputline. Was it really what you intended?

About your regex:

Don't nest character classes: e.g [[a-zA-Z0-9]\-\.] should be replaced with [-a-zA-Z0-9.] (you need to put - first in order to avoid it being confused with the interval separator, and . does not need to be escaped inside a character class).

Replacing your regex with /^(((http|https|ftp):\/\/)?([-a-zA-Z0-9.])+(\.)([a-zA-Z0-9]){2,4}([-a-zA-Z0-9+=%&_.~?\/]*))*$/ works for me.

RFC3986 Appendix B provides a better regex of course.