<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hello,<br>
<br>
looking at the patch, I see that the block for parsing first path
uri:<br>
<br>
<pre wrap="">+ if (parse_uri(path_dst.s, path_dst.len, &path_uri) < 0){
+ LM_ERR("failed to parse the Path URI\n");
+ ret = -3;
+ goto done;
+ }
Is done outside of parameter check:
+ if (path_check_local > 0</pre>
<br>
But seems to be used only inside it (when the parameter is set
>0). It should be moved inside that IF, to avoid parsing the path
uri when the parameter is not set, because that happens for each
lookup.<br>
<br>
Another thing that has to be double-checked: you change the value of
ptr->path.s if there is a match of local URI for first path. That
can mess the structure from usrloc, if it needs to do a free later
or is a pointer inside shared memory that is going to be used later
-- practically the pointer is no longer at the beginning of
allocated memory. I didn't have time to look deeper in usrloc and
all its db modes (iirc, for db-only, the location record is
temporarily built in memory and freed). The best and safest for the
future is to make a copy of the path str and work with it inside the
function where you need to change it<br>
<br>
str path_str;<br>
<br>
...<br>
path_str = ptr->path;<br>
// and use path_str instead of ptr->path from here on<br>
...<br>
<br>
<br>
The new parameter has to be documented in the readme as well.<br>
<br>
Cheers,<br>
Daniel<br>
<br>
<div class="moz-cite-prefix">On 11/12/14 14:10, Charles Chance
wrote:<br>
</div>
<blockquote
cite="mid:CAOvxgzAgA5xhZ28zVCNvVqzWX1=tjG9an2wy+_vE9PQaXqPViw@mail.gmail.com"
type="cite">
<div dir="ltr">Ok, no worries - thanks, Daniel.
<div><br>
</div>
<div>Quick note - I spotted that parse_uri() is not necessary
unless path_check_local is enabled, so that has been moved
now.<br>
</div>
<div><br>
</div>
<div>Otherwise, it is working nicely in one of our
installations.</div>
<div><br>
</div>
<div>Best regards,</div>
<div><br>
</div>
<div>Charles</div>
<div><br>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On 11 December 2014 at 12:52,
Daniel-Constantin Mierla <span dir="ltr"><<a
moz-do-not-send="true" href="mailto:miconda@gmail.com"
target="_blank">miconda@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> I didn't get the
chance to look at it yet -- it is on my radar.<br>
<br>
Cheers,<br>
Daniel
<div>
<div><br>
<br>
<div>On 11/12/14 11:24, Charles Chance wrote:<br>
</div>
</div>
</div>
<blockquote type="cite">
<div>
<div>
<p dir="ltr">Any further thoughts on this, please?</p>
<p dir="ltr">Cheers,</p>
<p dir="ltr">Charles<br>
</p>
<div style="margin:0 0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div dir="ltr">Hi Daniel,
<div><br>
</div>
<div>Please see attached (I have tested
already).</div>
<div><br>
</div>
<div>Should it be added, or left up to the
user to perform via config? Either way, at
least the patch is available here as an
option, in case the question is asked again.</div>
<div><br>
</div>
<div>It would be good to hear others'
opinions, too.</div>
<div><br>
</div>
<div>Best regards,</div>
<div><br>
</div>
<div>Charles</div>
<div><br>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On 8 December 2014
at 20:10, Charles Chance <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:charles.chance@sipcentric.com"
target="_blank">charles.chance@sipcentric.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote"
style="margin:0 0 0 .8ex;border-left:1px
#ccc solid;padding-left:1ex">
<p dir="ltr">Hi Daniel,</p>
<p dir="ltr">You are right, it is not
trivial to perform via config.</p>
<p dir="ltr">The patch was made quickly
to illustrate a point, but I have
reworked it now to include a parameter
for enabling the check, as well as
accounting for more than one Path
header.</p>
<p dir="ltr">If you think it is
worthwhile, I will post the full patch
for review tomorrow.</p>
<p dir="ltr">Best regards,</p>
<p dir="ltr">Charles<br>
</p>
<div class="gmail_quote">
<div>
<div>On 8 Dec 2014 16:28,
"Daniel-Constantin Mierla" <<a
moz-do-not-send="true"
href="mailto:miconda@gmail.com"
target="_blank">miconda@gmail.com</a>>
wrote:<br type="attribution">
</div>
</div>
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div>
<div>
<div bgcolor="#FFFFFF"
text="#000000"> <br>
<div>On 08/12/14 16:40,
Charles Chance wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On
8 December 2014 at
15:09, Olle E.
Johansson <span
dir="ltr"><<a
moz-do-not-send="true"
href="mailto:oej@edvina.net" target="_blank">oej@edvina.net</a>></span>
wrote:<br>
<blockquote
class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px
#ccc
solid;padding-left:1ex">
<div
style="word-wrap:break-word"><br>
<div><span>
<div>On 08 Dec
2014, at
16:00, Charles
Chance <<a
moz-do-not-send="true" href="mailto:charles.chance@sipcentric.com"
target="_blank">charles.chance@sipcentric.com</a>>
wrote:</div>
<br>
<blockquote
type="cite">
<div dir="ltr">
<div>Hi Olle,</div>
<div><br>
</div>
msg_apply_changes()
is for getting
the Path saved
the first
place if
adding/saving
on the same
instance.
<div><br>
</div>
<div>My patch
is for later
on, to avoid
looping if
lookup is
performed on
the same
instance that
received the
register.</div>
<div><br>
</div>
<div>Scenario
is 2 x
registrar/location
servers, both
sharing common
DB - no
separate edge
proxies, but
each adds
itself as Path
before saving
(which is
where
msg_apply_changes()
comes in).</div>
</div>
</blockquote>
</span>Can't you
sort that out in
the routing
script? I don't
see why we need
to add this in
the code... </div>
<div><br>
</div>
<div>If the
topmost,
leftmost routing
header in the
outbound INVITE
points to me,
remove it and
move on.</div>
<div>You have the
branch route for
that kind of
manipulation.</div>
<div><br>
</div>
<div>What am I
missing?</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
<br>
If I got it right upon quick
read, this case is not trivial
to handle via config file --
i.e., it is about saving
registration with local
address as a Path, the
registration can be read by
same proxy or another one (the
other will have to send the
register to this instance,
this one will need to ignore
the path).<br>
<br>
After lookup("location"), the
first Path appears as outbound
proxy address ($du / dst_uri),
but it is also added in the
lumps to be a Route header for
outgoing INVITE. If there are
more than on Path header,
things can get quite complex
to handle from config and
might be easier to simplify by
adding a module parameter to
enable/disable the proposed
patch.<br>
<br>
Cheers,<br>
Daniel<br>
<pre cols="72">--
Daniel-Constantin Mierla
<a moz-do-not-send="true" href="http://twitter.com/#%21/miconda" target="_blank">http://twitter.com/#!/miconda</a> - <a moz-do-not-send="true" href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/miconda</a></pre>
</div>
<br>
</div>
</div>
<span>_______________________________________________<br>
sr-dev mailing list<br>
<a moz-do-not-send="true"
href="mailto:sr-dev@lists.sip-router.org"
target="_blank">sr-dev@lists.sip-router.org</a><br>
<a moz-do-not-send="true"
href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev"
target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
<br>
</span></blockquote>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
<br>
</div>
</div>
<span><font face="Helvetica, Arial, sans-serif"><font><span
style="font-size:10pt"><a
moz-do-not-send="true"
href="http://www.sipcentric.com/"
title="blocked::http://www.sipcentric.com/"
target="_blank">www.sipcentric.com</a><br>
<br>
Follow us on twitter <a
moz-do-not-send="true"
href="http://twitter.com/sipcentric"
title="blocked::http://twitter.com/sipcentric"
target="_blank">@sipcentric</a><br>
<br>
<font color="gray">Sipcentric Ltd. Company
registered in England & Wales no.
7365592.</font> <font color="gray">Registered
office: Faraday Wharf, Innovation Birmingham
Campus, Holt Street, Birmingham Science
Park, Birmingham B7 4BB.</font></span></font></font>
<br>
<fieldset></fieldset>
<br>
</span><span>
<pre>_______________________________________________
sr-dev mailing list
<a moz-do-not-send="true" href="mailto:sr-dev@lists.sip-router.org" target="_blank">sr-dev@lists.sip-router.org</a>
<a moz-do-not-send="true" href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a>
</pre>
</span></blockquote>
<span> <br>
<pre cols="72">--
Daniel-Constantin Mierla
<a moz-do-not-send="true" href="http://twitter.com/#%21/miconda" target="_blank">http://twitter.com/#!/miconda</a> - <a moz-do-not-send="true" href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/miconda</a></pre>
</span></div>
<br>
_______________________________________________<br>
sr-dev mailing list<br>
<a moz-do-not-send="true"
href="mailto:sr-dev@lists.sip-router.org"
target="_blank">sr-dev@lists.sip-router.org</a><br>
<a moz-do-not-send="true"
href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev"
target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
<br>
<font face="Helvetica, Arial, sans-serif"><font size="2"><span
style="font-size:10pt"><a moz-do-not-send="true"
href="http://www.sipcentric.com/"
title="blocked::http://www.sipcentric.com/"
target="_blank">www.sipcentric.com</a><br>
<br>
Follow us on twitter <a moz-do-not-send="true"
href="http://twitter.com/sipcentric"
title="blocked::http://twitter.com/sipcentric"
target="_blank">@sipcentric</a><br>
<br>
<font color="gray">Sipcentric Ltd. Company registered in
England & Wales no. 7365592.</font> <font color="gray">Registered
office: Faraday Wharf, Innovation Birmingham Campus, Holt
Street, Birmingham Science Park, Birmingham B7 4BB.</font></span></font></font>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Daniel-Constantin Mierla
<a class="moz-txt-link-freetext" href="http://twitter.com/#!/miconda">http://twitter.com/#!/miconda</a> - <a class="moz-txt-link-freetext" href="http://www.linkedin.com/in/miconda">http://www.linkedin.com/in/miconda</a></pre>
</body>
</html>