[reSIProcate] Request for symbols definition (WS and WSS)

[Scott Godin]

Just to be clear - the original intent of the Symbols definitions was for
use in the SipMessage parsing logic.  These define many of the BNF tokens
we expect to see when parsing a SipMessage.

Scott


On Tue, Sep 17, 2013 at 12:20 PM, Scott Godin <sgodin at sipspectrum.com>wrote:

> I think having those symbols in there is good.  They don't necessarily
> need to be used by an application.  The stack itself should be using these
> symbols (this part needs work).  It provides a slight footprint
> optimization over using static "WS" strings throughout the code.
>
> Scott
>
>
> On Tue, Sep 17, 2013 at 12:10 PM, Daniel Pocock <daniel at pocock.com.au>wrote:
>
>> On 17/09/13 17:46, Dario Bozzali wrote:
>> > Hi all,
>> > Sorry to bother you (again) about WS and WSS, but I noticed that in
>> isDgramTransport function, used in TransportSelector::getFirstInterface()
>> method (file TransportSelector.cxx), cases for WS ans WSS are missing.
>> > I use WIN32 so I didn't faced issues, maybe in other configurations it
>> could be a problem.
>> > Best regards,
>> > Dario.
>>
>> Thanks for this feedback, I've committed a fix for that.  After we
>> branch v1.9, that code could move to TransportType, so I've added a
>> FIXME as well.
>>
>> For the Symbols::WS and Symbols::WSS, I decided to include them as well,
>> for consistency with the other symbols entries, even if they should be
>> deprecated - could anybody else provide any feedback about that?
>>
>> However, when looking at it again, another possible issue appeared.
>> Notice that Symbols::Sip represents the URI prefix "sip"?  To follow
>> this pattern, we would need
>>
>> Symbols::WS = "WS";    // transport, as in transport=WS
>> Symbols::Ws = "ws";    // URI prefix, as in ws://example.org
>>
>> so it is better to encapsulate these symbols in their own types (e.g.
>> TransportType).  I put in the Symbols::WS and WSS, but not the prefixes
>> for now.
>>
>> >
>> >
>> > bool isDgramTransport (TransportType type)
>> > {
>> >    static const bool unknown_transport = false;
>> >    switch(type)
>> >    {
>> >       case UDP:
>> >       case DTLS:
>> >       case DCCP:
>> >       case SCTP:
>> >          return   true;
>> >
>> >       case TCP:
>> >       case TLS:
>> > +++   case WS:
>> > +++   case WSS:
>> >          return   false;
>> >
>> >       default:
>> >          assert(unknown_transport);
>> >          return unknown_transport;  // !kh! just to make it compile
>> wo/warning.
>> >    }
>> > }
>> >
>> >
>> > -----Original Message-----
>> > From: resiprocate-devel-bounces at resiprocate.org [mailto:
>> resiprocate-devel-bounces at resiprocate.org] On Behalf Of Daniel Pocock
>> > Sent: lunedì 16 settembre 2013 20.20
>> > To: resiprocate-devel at resiprocate.org
>> > Subject: Re: [reSIProcate] Request for symbols definition (WS and WSS)
>> >
>> >
>> >
>> > On 16/09/13 18:56, Dario Bozzali wrote:
>> >> Hi all,
>> >>
>> >> It would be possible to add to mainline the definitions for symbols WS
>> >> and WSS (see below)?
>> >>
>> >
>> > I remember looking at that code when doing the WebRTC work but deciding
>> not to include WS and WSS in Symbols.?xx - I think the reason I didn't
>> include them is because I couldn't find other code explicitly referring to
>> these strings and it seemed better for people to use rutil/TransportType
>> >
>> > Can anybody else comment on these definitions?  Should they be
>> supported or are they deprecated?
>> >
>> >
>> >>
>> >>
>> >> Symbols.hxx:
>> >>
>> >> static const char* WS;
>> >>
>> >> static const char* WSS;
>> >>
>> >>
>> >>
>> >> Symbols.cxx:
>> >>
>> >> const char* Symbols::WS = "WS";
>> >>
>> >> const char* Symbols::WSS = "WSS";
>> >>
>> >>
>> >>
>> >> Moreover, in my opinion in method TransportType::isSecure() WS and WSS
>> >> transports should be handled (see below).
>> >>
>> >
>> >
>> > It looks like something went wrong with those changes when the b-webrtc
>> branch was merged.
>> >
>> > 10079 fixes isSecure, I have cherry-picked it from b-webrtc into main
>> >
>> > SVN believed the change was already merged, I had to force it to cherry
>> pick this with --ignore-ancestry
>> >
>> > svn merge --ignore-ancestry -c 10079
>> > https://svn.resiprocate.org/rep/resiprocate/branches/b-webrtc .
>> >
>> > Thanks for the feedback about these issues
>> >
>> > Regards,
>> >
>> > Daniel
>> > _______________________________________________
>> > resiprocate-devel mailing list
>> > resiprocate-devel at resiprocate.org
>> > https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>> >
>> > _______________________________________________
>> > resiprocate-devel mailing list
>> > resiprocate-devel at resiprocate.org
>> > https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>> _______________________________________________
>> resiprocate-devel mailing list
>> resiprocate-devel at resiprocate.org
>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20130917/1e9c4176/attachment.htm>