Skip Menu |

This queue is for tickets about the Net-UCP CPAN distribution.

Report information
The Basics
Id: 46467
Status: resolved
Worked: 10 min
Priority: 0/
Queue: Net-UCP

People
Owner: nemux [...] cpan.org
Requestors: SREZIC [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 0.39
Fixed in: (no value)



Subject: Possible problem with greedy regexp
The regexps in the parse_message method are possibly problematic, because the .* is greedy and might "eat" too much of the strings, leading to wrong matches. I think this should be fixed, see the patch in the attachment. Regards, Slaven
Subject: 0001-made-the-regexp-in-parse_message-non-greedy.patch
From d84e95d3bd82613e3adaf4063edb31132b64936b Mon Sep 17 00:00:00 2001 From: Slaven Rezic <slaven@rezic.de> Date: Fri, 29 May 2009 14:39:52 +0200 Subject: [PATCH 1/2] made the regexp in parse_message non-greedy --- lib/Net/UCP.pm | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/Net/UCP.pm b/lib/Net/UCP.pm index 2f5bd46..3a4f913 100644 --- a/lib/Net/UCP.pm +++ b/lib/Net/UCP.pm @@ -515,7 +515,7 @@ sub parse_message { my $ref_mess = undef; - if ($resp =~ m/^\d{2}\/\d{5}\/.*\/01\/.*/) { $ref_mess = $self->parse_01($resp) } + if ($resp =~ m/^\d{2}\/\d{5}\/.*?\/01\/.*/) { $ref_mess = $self->parse_01($resp) } elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/02\/.*/) { $ref_mess = $self->parse_02($resp) } elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/03\/.*/) { $ref_mess = $self->parse_03($resp) } elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/30\/.*/) { $ref_mess = $self->parse_30($resp) } -- 1.6.3.1
On Fri May 29 09:09:30 2009, SREZIC wrote: Show quoted text
> The regexps in the parse_message method are possibly problematic, > because the .* is greedy and might "eat" too much of the strings, > leading to wrong matches. I think this should be fixed, see the patch in > the attachment.
Additionally, the code could be simplified by using just one regexp, like in the next patch. Regards, Slaven
From d11f0c3c81a37f85e40ff8cd0f4441677113f101 Mon Sep 17 00:00:00 2001 From: Slaven Rezic <slaven@rezic.de> Date: Fri, 29 May 2009 14:45:02 +0200 Subject: [PATCH 2/2] simplified parse_message by using just one condition and one regexp --- lib/Net/UCP.pm | 19 ++++--------------- 1 files changed, 4 insertions(+), 15 deletions(-) diff --git a/lib/Net/UCP.pm b/lib/Net/UCP.pm index 3a4f913..3ca058b 100644 --- a/lib/Net/UCP.pm +++ b/lib/Net/UCP.pm @@ -515,21 +515,10 @@ sub parse_message { my $ref_mess = undef; - if ($resp =~ m/^\d{2}\/\d{5}\/.*?\/01\/.*/) { $ref_mess = $self->parse_01($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/02\/.*/) { $ref_mess = $self->parse_02($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/03\/.*/) { $ref_mess = $self->parse_03($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/30\/.*/) { $ref_mess = $self->parse_30($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/31\/.*/) { $ref_mess = $self->parse_31($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/51\/.*/) { $ref_mess = $self->parse_51($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/52\/.*/) { $ref_mess = $self->parse_52($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/53\/.*/) { $ref_mess = $self->parse_53($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/54\/.*/) { $ref_mess = $self->parse_54($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/55\/.*/) { $ref_mess = $self->parse_55($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/56\/.*/) { $ref_mess = $self->parse_56($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/57\/.*/) { $ref_mess = $self->parse_57($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/58\/.*/) { $ref_mess = $self->parse_58($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/60\/.*/) { $ref_mess = $self->parse_60($resp) } - elsif ($resp =~ m/^\d{2}\/\d{5}\/.*\/61\/.*/) { $ref_mess = $self->parse_61($resp) } + if (my($optype) = $resp =~ m{^\d{2}/\d{5}/.*?/(01|02|03|30|51|52|53|54|55|56|57|58|60|61)/.*}) { + my $parse_method = "parse_$optype"; + $ref_mess = $self->$parse_method($resp); + } return $ref_mess; } -- 1.6.3.1
On Fri May 29 09:11:42 2009, SREZIC wrote: Show quoted text
> On Fri May 29 09:09:30 2009, SREZIC wrote:
> > The regexps in the parse_message method are possibly problematic, > > because the .* is greedy and might "eat" too much of the strings, > > leading to wrong matches. I think this should be fixed, see the patch in > > the attachment.
> > Additionally, the code could be simplified by using just one regexp, > like in the next patch.
Sorry, I made a mistake. OT31 is missing now in parse_message. Please apply the attached patch to fix this. Regards, Slaven
diff --git a/lib/Net/UCP.pm b/lib/Net/UCP.pm index 1de258e..345dae5 100644 --- a/lib/Net/UCP.pm +++ b/lib/Net/UCP.pm @@ -515,7 +515,7 @@ sub parse_message { my $ref_mess = undef; - if (my($optype) = $resp =~ m{^\d{2}/\d{5}/.*?/(01|02|03|30|51|52|53|54|55|56|57|58|60|61)/.*}) { + if (my($optype) = $resp =~ m{^\d{2}/\d{5}/.*?/(01|02|03|30|31|51|52|53|54|55|56|57|58|60|61)/.*}) { my $parse_method = "parse_$optype"; $ref_mess = $self->$parse_method($resp); }
That's ok mate!