< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index  

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


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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Daniel Pocock
> Sent: lunedì 16 settembre 2013 20.20
> To: resiprocate-devel@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel

_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel