[OpenSER-Devel] [ openser-Bugs-1819248 ] replace_all, replace_body_all, subst anchor bug

[SourceForge.net]

Bugs item #1819248, was opened at 2007-10-24 13:54
Message generated for change (Comment added) made by bergolth
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1819248&group_id=139143

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: core
Group: ver 1.2.x
Status: Closed
Resolution: Accepted
Priority: 5
Private: No
Submitted By: Alexander Bergolth (bergolth)
Assigned to: Bogdan (bogdan_iancu)
Summary: replace_all, replace_body_all, subst anchor bug

Initial Comment:
Hi!

The replace_all, replace_body_all and subst functions all fail to match the beginning of the next line, when the last match ended with a newline.

e.g. when trying to remove the three lines
-------------------- 8< --------------------
a=alt:1 3 : xmv2v5mo ix7lpuPK 137.208.90.150 13726.
a=alt:2 2 : lPFc4bEV udepPIjW 192.168.217.1 13726.
a=alt:3 1 : 4o1caMp2 9P6XBBmd 192.168.172.1 13726.
-------------------- 8< --------------------

... replace_body_all("^a=alt:.*\r\n","") will remove only the first and the third line, it won't match the second line.

The reason is that after the first match, eflags|=REG_NOTBOL is set, so that in all subsequent matches, "^" won't match the beginning of the string. Unfortunately, since the last match ended with a newline, "^" _should_ match at the beginning of the string in this case.

Maybe the REG_NOTBOL flag should be set depending on the last character before the next match starts?

This bug is also present in subst_run() of re.c in the openser core.

I've attached a patch that fixes the problem for me.
(Not very well tested.)

Cheers,
--leo


----------------------------------------------------------------------

>Comment By: Alexander Bergolth (bergolth)
Date: 2007-11-12 15:47

Message:
Logged In: YES 
user_id=24695
Originator: YES

Hi!

Sorry for the late reply.
I did that initial position check, because I do think it's possible to
have null-string matches.
How about a regex like "x?" or "(x|)"?

At least with pcre, you have to explicitly disallow null-matches, see
PCRE_NOTEMPTY.

But if null-matches are really possible, all regexp-functions that will
continue where the last match ended, will loop. So maybe we should
explicitly handle that case and check if a match is empty?

Cheers,
--leo


----------------------------------------------------------------------

Comment By: Bogdan (bogdan_iancu)
Date: 2007-11-12 15:17

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Alexander,

I pushed your patch on SVN with some small changes - see previous
comment.

Thanks and regards,
Bogdan

----------------------------------------------------------------------

Comment By: Bogdan (bogdan_iancu)
Date: 2007-11-09 11:53

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Alexander,

I was reviewing the patch and it looks correct for me excepting one thing:
when setting/resetting the flag you test to see if the position is not the
start one: "if (p==input || *(p-1)=='\n')" . But I think p cannot equal
input anytime as this test is done after the first run of regexec and p
will point at the end of the matched string (which cannot be empty). So
p!=input all the time.....But maybe I'm missing something...:)

Thanks and regards,
Bogdan

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1819248&group_id=139143