SF.net SVN: opensips:[5933] branches/1.5

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

SF.net SVN: opensips:[5933] branches/1.5

Bogdan-Andrei Iancu
Revision: 5933
          http://opensips.svn.sourceforge.net/opensips/?rev=5933&view=rev
Author:   bogdan_iancu
Date:     2009-08-04 16:45:55 +0000 (Tue, 04 Aug 2009)

Log Message:
-----------
backport from trunk (rev #5916):
- fixed setting default values for SIP port and proto when greaping for a socket; the function ignore the port / proto if zero, but the parsing functions set them to 0 if they do not exist (in uris).

  Closes bug id #2827960
  Credits go to Peter Baer.

Revision Links:
--------------
    http://opensips.svn.sourceforge.net/opensips/?rev=5916&view=rev

Modified Paths:
--------------
    branches/1.5/modules/nat_traversal/nat_traversal.c
    branches/1.5/modules/registrar/save.c
    branches/1.5/modules/rr/loose.c
    branches/1.5/modules/tm/mi.c
    branches/1.5/pvar.c
    branches/1.5/socket_info.h


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

_______________________________________________
Devel mailing list
[hidden email]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
Reply | Threaded
Open this post in threaded view
|

Re: SF.net SVN: opensips:[5933] branches/1.5

Dan Pascu
On Tuesday 04 August 2009, Bogdan-Andrei Iancu wrote:

> Revision: 5933
>           http://opensips.svn.sourceforge.net/opensips/?rev=5933&view=rev
> Author:   bogdan_iancu
> Date:     2009-08-04 16:45:55 +0000 (Tue, 04 Aug 2009)
>
> Log Message:
> -----------
> backport from trunk (rev #5916):
> - fixed setting default values for SIP port and proto when greaping for a
> socket; the function ignore the port / proto if zero, but the parsing
> functions set them to 0 if they do not exist (in uris).

Wouldn't it be better if the parsing function would return the correct default
ports based on protocol, when missing, instead of returning 0 and requiring
the user to call another macro after that just to fix it?

>
>   Closes bug id #2827960
>   Credits go to Peter Baer.
>
> Revision Links:
> --------------
>     http://opensips.svn.sourceforge.net/opensips/?rev=5916&view=rev
>
> Modified Paths:
> --------------
>     branches/1.5/modules/nat_traversal/nat_traversal.c
>     branches/1.5/modules/registrar/save.c
>     branches/1.5/modules/rr/loose.c
>     branches/1.5/modules/tm/mi.c
>     branches/1.5/pvar.c
>     branches/1.5/socket_info.h
>
>
> This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
>
> _______________________________________________
> Devel mailing list
> [hidden email]
> http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
>
>


--
Dan

_______________________________________________
Devel mailing list
[hidden email]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
Reply | Threaded
Open this post in threaded view
|

Re: SF.net SVN: opensips:[5933] branches/1.5

Bogdan-Andrei Iancu
I thought of this in the first thing, but (as documented), if the the
port or proto is missing, the grep function assumes ANY and not a
default value. And actually the function is used in other part with the
ANY behaviour.

Regards,
Bogdan

Dan Pascu wrote:

> On Tuesday 04 August 2009, Bogdan-Andrei Iancu wrote:
>  
>> Revision: 5933
>>           http://opensips.svn.sourceforge.net/opensips/?rev=5933&view=rev
>> Author:   bogdan_iancu
>> Date:     2009-08-04 16:45:55 +0000 (Tue, 04 Aug 2009)
>>
>> Log Message:
>> -----------
>> backport from trunk (rev #5916):
>> - fixed setting default values for SIP port and proto when greaping for a
>> socket; the function ignore the port / proto if zero, but the parsing
>> functions set them to 0 if they do not exist (in uris).
>>    
>
> Wouldn't it be better if the parsing function would return the correct default
> ports based on protocol, when missing, instead of returning 0 and requiring
> the user to call another macro after that just to fix it?
>
>  
>>   Closes bug id #2827960
>>   Credits go to Peter Baer.
>>
>> Revision Links:
>> --------------
>>     http://opensips.svn.sourceforge.net/opensips/?rev=5916&view=rev
>>
>> Modified Paths:
>> --------------
>>     branches/1.5/modules/nat_traversal/nat_traversal.c
>>     branches/1.5/modules/registrar/save.c
>>     branches/1.5/modules/rr/loose.c
>>     branches/1.5/modules/tm/mi.c
>>     branches/1.5/pvar.c
>>     branches/1.5/socket_info.h
>>
>>
>> This was sent by the SourceForge.net collaborative development platform, the
>>    
> world's largest Open Source development site.
>  
>> _______________________________________________
>> Devel mailing list
>> [hidden email]
>> http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
>>
>>
>>    
>
>
>  


_______________________________________________
Devel mailing list
[hidden email]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
Reply | Threaded
Open this post in threaded view
|

Re: SF.net SVN: opensips:[5933] branches/1.5

Dan Pascu
On Tuesday 04 August 2009, Bogdan-Andrei Iancu wrote:
> I thought of this in the first thing, but (as documented), if the the
> port or proto is missing, the grep function assumes ANY and not a
> default value. And actually the function is used in other part with the
> ANY behaviour.

Anyway, in the case of the nat_traversal module, the fix is only if one wants
to be extremely cautious. The values the nat_traversal reads are the ones that
were written by itself at shutdown. The only way to read something invalid
back is if the user modifies it, however the file has an explicit and strong
warning against being modified. Finally, if one ignores the warning and still
modifies the file, then I don't think that adding the default port would help
in any way, as if the socket address may not be right (port may be different),
NAT cannot be traversed back, so I think the original behavior or ignoring
that entry because it doesn't match any entry in the socket list is better
than trying to send from the wrong port, which would not work anyway.

>
> Regards,
> Bogdan
>
> Dan Pascu wrote:
> > On Tuesday 04 August 2009, Bogdan-Andrei Iancu wrote:
> >  
> >> Revision: 5933
> >>           http://opensips.svn.sourceforge.net/opensips/?rev=5933&view=rev
> >> Author:   bogdan_iancu
> >> Date:     2009-08-04 16:45:55 +0000 (Tue, 04 Aug 2009)
> >>
> >> Log Message:
> >> -----------
> >> backport from trunk (rev #5916):
> >> - fixed setting default values for SIP port and proto when greaping for a
> >> socket; the function ignore the port / proto if zero, but the parsing
> >> functions set them to 0 if they do not exist (in uris).
> >>    
> >
> > Wouldn't it be better if the parsing function would return the correct
default
> > ports based on protocol, when missing, instead of returning 0 and
requiring

> > the user to call another macro after that just to fix it?
> >
> >  
> >>   Closes bug id #2827960
> >>   Credits go to Peter Baer.
> >>
> >> Revision Links:
> >> --------------
> >>     http://opensips.svn.sourceforge.net/opensips/?rev=5916&view=rev
> >>
> >> Modified Paths:
> >> --------------
> >>     branches/1.5/modules/nat_traversal/nat_traversal.c
> >>     branches/1.5/modules/registrar/save.c
> >>     branches/1.5/modules/rr/loose.c
> >>     branches/1.5/modules/tm/mi.c
> >>     branches/1.5/pvar.c
> >>     branches/1.5/socket_info.h
> >>
> >>
> >> This was sent by the SourceForge.net collaborative development platform,
the

> >>    
> > world's largest Open Source development site.
> >  
> >> _______________________________________________
> >> Devel mailing list
> >> [hidden email]
> >> http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
> >>
> >>
> >>    
> >
> >
> >  
>
>


--
Dan

_______________________________________________
Devel mailing list
[hidden email]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
Reply | Threaded
Open this post in threaded view
|

Re: SF.net SVN: opensips:[5933] branches/1.5

Bogdan-Andrei Iancu
Dan Pascu wrote:

> On Tuesday 04 August 2009, Bogdan-Andrei Iancu wrote:
>  
>> I thought of this in the first thing, but (as documented), if the the
>> port or proto is missing, the grep function assumes ANY and not a
>> default value. And actually the function is used in other part with the
>> ANY behaviour.
>>    
>
> Anyway, in the case of the nat_traversal module, the fix is only if one wants
> to be extremely cautious.
Correct - even if the data is generated and used by the modules, the
fact that is stored somewhere it can be changed (like DB, files, etc),
just justify a check - anyhow it is a very simple check.

> The values the nat_traversal reads are the ones that
> were written by itself at shutdown. The only way to read something invalid
> back is if the user modifies it, however the file has an explicit and strong
> warning against being modified. Finally, if one ignores the warning and still
> modifies the file, then I don't think that adding the default port would help
> in any way, as if the socket address may not be right (port may be different),
> NAT cannot be traversed back, so I think the original behavior or ignoring
> that entry because it doesn't match any entry in the socket list is better
> than trying to send from the wrong port, which would not work anyway.
>  
assuming the data was altered, without the fix, a totally different
socket could be used (see the example with RR in the bug report). It is
not about not pinging, but pinging from the wrong socket.

Regards,
Bogdan

_______________________________________________
Devel mailing list
[hidden email]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
Reply | Threaded
Open this post in threaded view
|

Re: SF.net SVN: opensips:[5933] branches/1.5

Dan Pascu
On Wednesday 05 August 2009, Bogdan-Andrei Iancu wrote:
> > NAT cannot be traversed back, so I think the original behavior or ignoring
> > that entry because it doesn't match any entry in the socket list is better
> > than trying to send from the wrong port, which would not work anyway.
> >  
> assuming the data was altered, without the fix, a totally different
> socket could be used (see the example with RR in the bug report). It is
> not about not pinging, but pinging from the wrong socket.

That's my point. If the data was altered it'll ping from the wrong socket
anyway, fix or no fix.

P.S. I'm not advocating that it should be reverted, I'm just highlighting that
it doesn't help much (if at all). There is a reason why the data file strongly
discourages people to edit it.

--
Dan

_______________________________________________
Devel mailing list
[hidden email]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel