[sip-comm-dev] [ice4j] [patch] MSN calls, first steps.


#1

Hi

patch-ice4j.txt (51.7 KB)

···

Le 08/07/2010 14:41, Emil Ivov a écrit :
> Lubomir just noted that there's no patch attached to the mail. I know a
> had a copy privately but do you mind sending it to the list?
>
Sorry, here it is :slight_smile:

(I was sure smthg like this would happen to me here...)

Regards,
Valentin


#2

Does the MsTurnCandidateHarvester utilize the STUN long-term
credential mechanism? I see its definition deals with
longTermCredential but then its "First Request" (the MSN Allocate
Request) adds a USERNAME attribute and the STUN long-term credential
mechanism says that the client SHOULD omit the USERNAME attribute from
the "First Request". My guess is that MsTurnCandidateHarvester doesn't
utilize the STUN long-term credential mechanism and the support for
LongTermCredential in it is related to the fact that
MsTurnCandidateHarvester is a copy-and-paste of
TurnCandidateHarvester.

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Valentin, I'm afraid I don't find patch-ice4j.txt to be ready for
committing. Apart from the MsTurnCandidateHarvester and Harvest which
you already agreed on not being meant to be present in the patch, I
found the following issues I'd like to see fixed:

- The newly introduced Attribute classes are pretty much
copy-and-paste of one and the same code with just the attribute name
changed where necessary. Apart from the unnecessarily long code,
another disadvantage of the copy-and-paste style of programming is
that one inherits the shortcomings of the original code. For example,
cloning the private arrays is done with 8 lines of code instead of 1
and one has to fix it in the 5 new Attribute classes.

- There's code that is not used for that is unnecessarily changed. For
example, a new class that extends Request and is never used. Another
example is changing a function to return SessionDescription instead of
String and then not add new uses of it and just change the old used to
call toString on the result.

- There are imports which import specific classes and we only import packages.

- The newly introduced Attribute classes lack javadocs to a great
extent and we require javadocs on everything.

- There is whitespace at the beginning of the empty lines. I guess
that's Eclipse to blame but it's obvious in the Compare Editor of the
Team Synchronizing perspective.

···

On Thu, Jul 8, 2010 at 2:57 PM, Valentin MARTINET <vmartinet.sipcommunicator@gmail.com> wrote:

here it is :slight_smile:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hi Lubomir,

Le 10/07/2010 15:47, Lubomir Marinov a �crit :

Does the MsTurnCandidateHarvester utilize the STUN long-term
credential mechanism?

It doesn't. MsTurnCandidateHarvester at his early stages was a quick way for me
to use and send my own messages which try to match MSN's ones. As it is currently
a copy-and-paste of TurnCandidateHarvester, there are effectively useless methods.

As I'm working on TURN/STUN haversters and harvests, I think it may be not necessary
to commit them. Actually, I thought they were not yet included in this patch :slight_smile:

Regards,
Valentin

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

I agree with you, I will review this patch and send a much more
updated version of it.
(Sorry for some issues that are mostly inattention from me, such as
package imports for instance)

Thanks for your remarks,

Regards,
Valentin

Le 10/07/2010 16:18, Lubomir Marinov a �crit :

···

On Thu, Jul 8, 2010 at 2:57 PM, Valentin MARTINET > <vmartinet.sipcommunicator@gmail.com> wrote:
   

here it is :slight_smile:
     

Valentin, I'm afraid I don't find patch-ice4j.txt to be ready for
committing. Apart from the MsTurnCandidateHarvester and Harvest which
you already agreed on not being meant to be present in the patch, I
found the following issues I'd like to see fixed:

- The newly introduced Attribute classes are pretty much
copy-and-paste of one and the same code with just the attribute name
changed where necessary. Apart from the unnecessarily long code,
another disadvantage of the copy-and-paste style of programming is
that one inherits the shortcomings of the original code. For example,
cloning the private arrays is done with 8 lines of code instead of 1
and one has to fix it in the 5 new Attribute classes.

- There's code that is not used for that is unnecessarily changed. For
example, a new class that extends Request and is never used. Another
example is changing a function to return SessionDescription instead of
String and then not add new uses of it and just change the old used to
call toString on the result.

- There are imports which import specific classes and we only import packages.

- The newly introduced Attribute classes lack javadocs to a great
extent and we require javadocs on everything.

- There is whitespace at the beginning of the empty lines. I guess
that's Eclipse to blame but it's obvious in the Compare Editor of the
Team Synchronizing perspective.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Hey Valentin,

На 10.07.10 16:07, Valentin MARTINET написа:

Hi Lubomir,

Does the MsTurnCandidateHarvester utilize the STUN long-term
credential mechanism?

It doesn't. MsTurnCandidateHarvester at his early stages was a quick way
for me
to use and send my own messages which try to match MSN's ones. As it is
currently
a copy-and-paste of TurnCandidateHarvester, there are effectively
useless methods.

As I'm working on TURN/STUN haversters and harvests

Normally there wouldn't be any need for your to work on those and you
should be able to use them as they are. Is there something that needs to
be changed because of MSN specifics?

Emil

···

Le 10/07/2010 15:47, Lubomir Marinov a écrit :

, I think it may be
not necessary
to commit them. Actually, I thought they were not yet included in this
patch :slight_smile:

Regards,
Valentin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#7

Hi Emil,

It doesn't. MsTurnCandidateHarvester at his early stages was a quick way
for me
to use and send my own messages which try to match MSN's ones. As it is
currently
a copy-and-paste of TurnCandidateHarvester, there are effectively
useless methods.

As I'm working on TURN/STUN haversters and harvests

Normally there wouldn't be any need for your to work on those and you
should be able to use them as they are. Is there something that needs to
be changed because of MSN specifics?

I forked the STUN and TURN Harvesters (+Harvests) in order to use the MsnMessageFactory that generates
MSN specific messages (instead of the original ice4j's MessageFactory that is used in Harvests).
It's currently the best way I've found.

Regards,
Valentin

···

Le 10/07/2010 17:13, Emil Ivov a écrit :

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#8

На 10.07.10 16:58, Valentin MARTINET написа:

Hi Emil,

It doesn't. MsTurnCandidateHarvester at his early stages was a quick way
for me
to use and send my own messages which try to match MSN's ones. As it is
currently
a copy-and-paste of TurnCandidateHarvester, there are effectively
useless methods.

As I'm working on TURN/STUN haversters and harvests

Normally there wouldn't be any need for your to work on those and you
should be able to use them as they are. Is there something that needs to
be changed because of MSN specifics?

I forked the STUN and TURN Harvesters (+Harvests) in order to use the
MsnMessageFactory that generates
MSN specific messages (instead of the original ice4j's MessageFactory
that is used in Harvests).

Could you please summarize the difference between the STUN binding and
allocation requests and those used by MSN?

It's currently the best way I've found.

Well, for all the reasons that Lubomir already pointed out, extending is
generally a better choice. More after you tell us about the syntax
differences in MSN's STUN/TURN variant.

Emil

···

Le 10/07/2010 17:13, Emil Ivov a écrit :

Regards,
Valentin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
SIP Communicator
emcho@sip-communicator.org PHONE: +33.1.77.62.43.30
http://sip-communicator.org FAX: +33.1.77.62.47.31

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#9

Most of the differences are in the fact that MSN requests use additionnal attributes that are specific
to Microsoft, such as "MS-Version", "MS-Service-Quality", and more. Moreover, in the documentation
I've found about MSN protocols, there are also requirements in the order of appearance of attributes inside
a message: for instance, in TURN allocate requests, there is a "Magic-Cookie" attribute that has to be the
first attribute of the message.

Hope this helps,

Regards,
Valentin

···

Le 10/07/2010 18:03, Emil Ivov a écrit :

Could you please summarize the difference between the STUN binding and
allocation requests and those used by MSN?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#10

Hello all,

Just for information for those who are interrested in MS-TURN. Here the link of the specification: http://download.microsoft.com/download/1/6/F/16F4E321-AA6B-4FA3-8AD3-E94C895A3C97/[MS-TURN].pdf

I have not read in details but it seems that it is based on an very old TURN draft (according to the references) dated from 2005: http://tools.ietf.org/html/draft-rosenberg-midcom-turn-08 that why there are some attributes like BANDWIDTH, MAGIC-COOKIE, ... which disapeared now in TURN RFC (5766).

Regards,

···

--
Seb

Le 10/07/2010 17:49, Valentin MARTINET a écrit :

Le 10/07/2010 18:03, Emil Ivov a écrit :

Could you please summarize the difference between the STUN binding and
allocation requests and those used by MSN?

Most of the differences are in the fact that MSN requests use additionnal attributes that are specific
to Microsoft, such as "MS-Version", "MS-Service-Quality", and more. Moreover, in the documentation
I've found about MSN protocols, there are also requirements in the order of appearance of attributes inside
a message: for instance, in TURN allocate requests, there is a "Magic-Cookie" attribute that has to be the
first attribute of the message.

Hope this helps,

Regards,
Valentin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net