[sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation


#1

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC
in cooperation with other SIMPLE clients, so I looked a little closer
into the presence implementation and found a few things to enhance
there. Attached is a patch for the
net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are
contained in the setPidfPresenceStatus() method, which handles the PIDF
documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting
changes I made:
* I was not able to see the presence status of clients other than SC,
even though SC received NOTIFY messages containing the PIDF documents
with presence information of these clients. The reason for this is the
missing "contact" element in the tuples of the PIDF documents. While the
contact element SHOULD be included in tuples according to RFC 3863, this
is not mandatory. Even though I'd also say contact should always be
present, some other clients I used (X-Lite, pidgin) do not include this
tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value
of the mandatory "entity" attribute of the root tag "presence" is
considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples
(usually, there should be at most one). This is now handled as with
several other tags - if multiple contact elements should be present for
one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml
namespaces. This had the effect that the RPID status was never actually
considered to change the presence state in the contact list. The visible
extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the
additional <note> element which should (as already stated in the code)
usually not be used to indicate detailed state information. The main
problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any
namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in
the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag");
(with ANY_NS being a wildcard "*" for an arbitrary namespace) for the
RPID handling. Then it doesn't matter to which namespace the element
belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the
standard namespace for the common pidf tags, the current implementation
would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the
calls to getElementsByTagName() by calls to getElementsByTagNameNS(),
using the namespace wildcard. However, I did _not_ do this yet, mainly
because I don't like these methods too much anyway. This is because they
do not only return immediate child elements for the given node, but also
elements in lower levels (further nested in the document tree). Not sure
if this is the ideal handling, so I left this up to discussion. I'd
prefer to use the getChildNodes() method to get the immediate children
for a node. What do you think?

Cheers,
Ralph Weires

sip-presence-pidf-processing.patch (13.9 KB)

···

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.


#2

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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


#3

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

···

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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 ralph,

I looked at all your propositions and at your patch. First, thank you very much for all the efforts you are spending in helping us to improve SC.

(more inline)

ralph.weires@hitec.lu a �crit :

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC
in cooperation with other SIMPLE clients, so I looked a little closer
into the presence implementation and found a few things to enhance
there. Attached is a patch for the
net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are
contained in the setPidfPresenceStatus() method, which handles the PIDF
documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting
changes I made:
* I was not able to see the presence status of clients other than SC,
even though SC received NOTIFY messages containing the PIDF documents
with presence information of these clients. The reason for this is the
missing "contact" element in the tuples of the PIDF documents. While the
contact element SHOULD be included in tuples according to RFC 3863, this
is not mandatory. Even though I'd also say contact should always be
present, some other clients I used (X-Lite, pidgin) do not include this
tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value
of the mandatory "entity" attribute of the root tag "presence" is
considered as contact address instead.
  
Yes, you're absolutely right, it's my bad. It seems that I just forget this case. Your patch correct it very well.

** There used to be a loop handling all contact elements in the tuples
(usually, there should be at most one). This is now handled as with
several other tags - if multiple contact elements should be present for
one tuple, only the last one is considered.
  
There was a special reason for this specific behavior so I'm not sure that your correction is a good idea here.
During the design, I thought that even if it's not allowed, some bad designed presence agent could one day send us a presence document for all the contacts with the same status. (Like one NOTIFY for all away clients, one for all busy, and so on...)
Of course this case is not correct (related to RFCs) and really dirty but as it costs us nearly nothing, I thought that it's better to handle this with a loop.

* There was a mistake in the RPID processing, which was caused by xml
namespaces. This had the effect that the RPID status was never actually
considered to change the presence state in the contact list. The visible
extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the
additional <note> element which should (as already stated in the code)
usually not be used to indicate detailed state information. The main
problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any
namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in
the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag");
(with ANY_NS being a wildcard "*" for an arbitrary namespace) for the
RPID handling. Then it doesn't matter to which namespace the element
belongs to.
  
You're certainly right, thanks for this catch and even more for your patch for it.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the
standard namespace for the common pidf tags, the current implementation
would not accept the document. Such as a document like this: <?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the
calls to getElementsByTagName() by calls to getElementsByTagNameNS(),
using the namespace wildcard. However, I did _not_ do this yet, mainly
because I don't like these methods too much anyway. This is because they
do not only return immediate child elements for the given node, but also
elements in lower levels (further nested in the document tree). Not sure
if this is the ideal handling, so I left this up to discussion. I'd
prefer to use the getChildNodes() method to get the immediate children
for a node. What do you think?
  
Using the wildcard namespace is not really justified I think because, in this case, we are sure of the namespace we want, so yes, there is effectively a possible problem but we may find another solution.

I'd like to have your opinion on this before applying your patch.
Anyway thanks for all this work !

Cheers,
Ben

···

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


#5

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

Yes, that's exactly what the recent patch should already fix.

···

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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 Ben,

- Concerning the loop for the contact elements: Ok, I didn't recognize the reason for explicitely dealing with all eventually contained contact elements. You have a point there, right. I removed the loop just because it prevented me from doubling code parts in the method. No problem to leave that as it was, I guess :slight_smile:
It's just that the additional handling of the information in <note>-tags (not desired by RFC, but sometimes used by clients as you mention in the code) currently only comes into play if there is at least one contact element. Perhaps it makes sense to put this part to the very beginning of the <tuple>-handling - it should give the same result for each contact element anyway. Additionally, by that way it can be used for the case of zero contact elements as well without duplicating the code.

- Regarding the general processing of xml namespaces: As long as the defined namespaces are more or less ignored, I think it would be good if SC would be able to accept the standard pidf tags also if they are not defined in the default namespace (i.e., with a prefix). You're right, we know in which namespace the tags are supposed to be, so we could use "urn:ietf:params:xml:ns:pidf" instead of the wildcard "*" in a getElementsByTagNameNS()-method to just get elements from the correct namespace. However, then we have a problem again in case that a pidf document doesn't contain correct namespace definitions at all...
Generally, a good solution IMHO would be a strict processing of the pidf documents by default (including the requirement for correct namespaces), with a fallback to some kind of "quirks" mode if any errors occur in the strict mode (similar to how several webbrowsers handle webpages).

Cheers,
Ralph

···

-----Original Message-----

From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr]

Sent: Monday, October 15, 2007 7:13 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi ralph,

I looked at all your propositions and at your patch. First, thank you very much for all the efforts you are spending in helping us to improve SC.

(more inline)

ralph.weires@hitec.lu a écrit :

Hi again,

I had some more problems with the SIMPLE (presence) functionality of
SC in cooperation with other SIMPLE clients, so I looked a little
closer into the presence implementation and found a few things to
enhance there. Attached is a patch for the
net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImp
l class which should correct some errors. Almost all performed changes
are contained in the setPidfPresenceStatus() method, which handles the
PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting
changes I made:
* I was not able to see the presence status of clients other than SC,
even though SC received NOTIFY messages containing the PIDF documents
with presence information of these clients. The reason for this is the
missing "contact" element in the tuples of the PIDF documents. While
the contact element SHOULD be included in tuples according to RFC
3863, this is not mandatory. Even though I'd also say contact should
always be present, some other clients I used (X-Lite, pidgin) do not
include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value
of the mandatory "entity" attribute of the root tag "presence" is
considered as contact address instead.
  
Yes, you're absolutely right, it's my bad. It seems that I just forget this case. Your patch correct it very well.

** There used to be a loop handling all contact elements in the tuples
(usually, there should be at most one). This is now handled as with
several other tags - if multiple contact elements should be present
for one tuple, only the last one is considered.
  
There was a special reason for this specific behavior so I'm not sure that your correction is a good idea here.
During the design, I thought that even if it's not allowed, some bad designed presence agent could one day send us a presence document for all the contacts with the same status. (Like one NOTIFY for all away clients, one for all busy, and so on...) Of course this case is not correct (related to RFCs) and really dirty but as it costs us nearly nothing, I thought that it's better to handle this with a loop.

* There was a mistake in the RPID processing, which was caused by xml
namespaces. This had the effect that the RPID status was never
actually considered to change the presence state in the contact list.
The visible extended state of other SC users in the contact list
(busy, away, on the
phone) was still adapted correctly, but this was just because of the
additional <note> element which should (as already stated in the code)
usually not be used to indicate detailed state information. The main
problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any
namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained
in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS
being a wildcard "*" for an arbitrary namespace) for the RPID
handling. Then it doesn't matter to which namespace the element
belongs to.
  
You're certainly right, thanks for this catch and even more for your patch for it.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the
standard namespace for the common pidf tags, the current
implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing
the calls to getElementsByTagName() by calls to
getElementsByTagNameNS(), using the namespace wildcard. However, I did
_not_ do this yet, mainly because I don't like these methods too much
anyway. This is because they do not only return immediate child
elements for the given node, but also elements in lower levels
(further nested in the document tree). Not sure if this is the ideal
handling, so I left this up to discussion. I'd prefer to use the
getChildNodes() method to get the immediate children for a node. What do you think?
  
Using the wildcard namespace is not really justified I think because, in this case, we are sure of the namespace we want, so yes, there is effectively a possible problem but we may find another solution.

I'd like to have your opinion on this before applying your patch.
Anyway thanks for all this work !

Cheers,
Ben

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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


#8

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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


#9

Hi Ralph, Joel, all,

I've commited a patch based on the Ralph's one.
Joel it should correct all your problems now. Wait for the next cruise control build and update.

Sadly we've got some problems with our sip provider for the moment so I'm not able to test deeply this patch, as I wanted to fix your problems quickly I've commited this anyway but know that this patch will perhaps need some corrections. If you still encounter difficulties with the new version, let us know please.

The patch consist in the Ralph's patch with the correction to handle every <contact> in a <tuple>, a correction to take into account the <note>s even if the document is not conform to the RFC and we now use the standard namespace to find PID informations.

I didn't added a fallback to a wildcard namespace for PID because I really dislike it. Using it with RPID was a necessity because it seems that many SIP clients aren't using the new namespace defined in the RFC recently. It's AFAIK not the case with the PIDF namespace so I prefer keep a strict conformance to RFC here until we find an exception to this. And even with an exception, it's theoretically the bad coded client which should be fixed.

Cheers,
Ben.

ralph.weires@hitec.lu a �crit :

···

Hi Ben,

- Concerning the loop for the contact elements: Ok, I didn't recognize the reason for explicitely dealing with all eventually contained contact elements. You have a point there, right. I removed the loop just because it prevented me from doubling code parts in the method. No problem to leave that as it was, I guess :slight_smile:
It's just that the additional handling of the information in <note>-tags (not desired by RFC, but sometimes used by clients as you mention in the code) currently only comes into play if there is at least one contact element. Perhaps it makes sense to put this part to the very beginning of the <tuple>-handling - it should give the same result for each contact element anyway. Additionally, by that way it can be used for the case of zero contact elements as well without duplicating the code.

- Regarding the general processing of xml namespaces: As long as the defined namespaces are more or less ignored, I think it would be good if SC would be able to accept the standard pidf tags also if they are not defined in the default namespace (i.e., with a prefix). You're right, we know in which namespace the tags are supposed to be, so we could use "urn:ietf:params:xml:ns:pidf" instead of the wildcard "*" in a getElementsByTagNameNS()-method to just get elements from the correct namespace. However, then we have a problem again in case that a pidf document doesn't contain correct namespace definitions at all...
Generally, a good solution IMHO would be a strict processing of the pidf documents by default (including the requirement for correct namespaces), with a fallback to some kind of "quirks" mode if any errors occur in the strict mode (similar to how several webbrowsers handle webpages).

Cheers,
Ralph

-----Original Message-----
From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr] Sent: Monday, October 15, 2007 7:13 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi ralph,

I looked at all your propositions and at your patch. First, thank you very much for all the efforts you are spending in helping us to improve SC.

(more inline)

ralph.weires@hitec.lu a �crit :
  

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImp
l class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
  
Yes, you're absolutely right, it's my bad. It seems that I just forget this case. Your patch correct it very well.

** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.
  
There was a special reason for this specific behavior so I'm not sure that your correction is a good idea here.
During the design, I thought that even if it's not allowed, some bad designed presence agent could one day send us a presence document for all the contacts with the same status. (Like one NOTIFY for all away clients, one for all busy, and so on...) Of course this case is not correct (related to RFCs) and really dirty but as it costs us nearly nothing, I thought that it's better to handle this with a loop.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.
  
You're certainly right, thanks for this catch and even more for your patch for it.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?
  
Using the wildcard namespace is not really justified I think because, in this case, we are sure of the namespace we want, so yes, there is effectively a possible problem but we may find another solution.

I'd like to have your opinion on this before applying your patch.
Anyway thanks for all this work !

Cheers,
Ben

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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


#10

Works fine, at least for me...

Ralph Weires

···

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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


#11

Hi again Ben,

I tried the changed version with some other clients and it looks good. The only of the clients whose presence now isn't recognised anymore is WengoPhone, but that's just due to the bad pidf implementation there - the documents of Wengo do not contain any xml namespace declaration, thus they don't meet the RFC requirements.

I think it's ok to be rather strict with the implementation - the most important thing is that the communication with clients behaving according to the RFCs works well.

Cheers,
Ralph

···

-----Original Message-----

From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr]

Sent: Tuesday, October 16, 2007 3:20 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Ralph, Joel, all,

I've commited a patch based on the Ralph's one.
Joel it should correct all your problems now. Wait for the next cruise control build and update.

Sadly we've got some problems with our sip provider for the moment so I'm not able to test deeply this patch, as I wanted to fix your problems quickly I've commited this anyway but know that this patch will perhaps need some corrections. If you still encounter difficulties with the new version, let us know please.

The patch consist in the Ralph's patch with the correction to handle every <contact> in a <tuple>, a correction to take into account the <note>s even if the document is not conform to the RFC and we now use the standard namespace to find PID informations.

I didn't added a fallback to a wildcard namespace for PID because I really dislike it. Using it with RPID was a necessity because it seems that many SIP clients aren't using the new namespace defined in the RFC recently. It's AFAIK not the case with the PIDF namespace so I prefer keep a strict conformance to RFC here until we find an exception to this. And even with an exception, it's theoretically the bad coded client which should be fixed.

Cheers,
Ben.

ralph.weires@hitec.lu a écrit :

Hi Ben,

- Concerning the loop for the contact elements: Ok, I didn't recognize
the reason for explicitely dealing with all eventually contained contact elements. You have a point there, right. I removed the loop just because it prevented me from doubling code parts in the method. No problem to leave that as it was, I guess :slight_smile: It's just that the additional handling of the information in <note>-tags (not desired by RFC, but sometimes used by clients as you mention in the code) currently only comes into play if there is at least one contact element. Perhaps it makes sense to put this part to the very beginning of the <tuple>-handling - it should give the same result for each contact element anyway. Additionally, by that way it can be used for the case of zero contact elements as well without duplicating the code.

- Regarding the general processing of xml namespaces: As long as the defined namespaces are more or less ignored, I think it would be good if SC would be able to accept the standard pidf tags also if they are not defined in the default namespace (i.e., with a prefix). You're right, we know in which namespace the tags are supposed to be, so we could use "urn:ietf:params:xml:ns:pidf" instead of the wildcard "*" in a getElementsByTagNameNS()-method to just get elements from the correct namespace. However, then we have a problem again in case that a pidf document doesn't contain correct namespace definitions at all...
Generally, a good solution IMHO would be a strict processing of the pidf documents by default (including the requirement for correct namespaces), with a fallback to some kind of "quirks" mode if any errors occur in the strict mode (similar to how several webbrowsers handle webpages).

Cheers,
Ralph

-----Original Message-----
From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr]
Sent: Monday, October 15, 2007 7:13 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP
presence implementation

Hi ralph,

I looked at all your propositions and at your patch. First, thank you very much for all the efforts you are spending in helping us to improve SC.

(more inline)

ralph.weires@hitec.lu a écrit :
  

Hi again,

I had some more problems with the SIMPLE (presence) functionality of
SC in cooperation with other SIMPLE clients, so I looked a little
closer into the presence implementation and found a few things to
enhance there. Attached is a patch for the
net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipIm
p l class which should correct some errors. Almost all performed
changes are contained in the setPidfPresenceStatus() method, which
handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the
resulting changes I made:
* I was not able to see the presence status of clients other than SC,
even though SC received NOTIFY messages containing the PIDF documents
with presence information of these clients. The reason for this is
the missing "contact" element in the tuples of the PIDF documents.
While the contact element SHOULD be included in tuples according to
RFC 3863, this is not mandatory. Even though I'd also say contact
should always be present, some other clients I used (X-Lite, pidgin)
do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the
value of the mandatory "entity" attribute of the root tag "presence"
is considered as contact address instead.
  
Yes, you're absolutely right, it's my bad. It seems that I just forget this case. Your patch correct it very well.

** There used to be a loop handling all contact elements in the
tuples (usually, there should be at most one). This is now handled as
with several other tags - if multiple contact elements should be
present for one tuple, only the last one is considered.
  
There was a special reason for this specific behavior so I'm not sure that your correction is a good idea here.
During the design, I thought that even if it's not allowed, some bad designed presence agent could one day send us a presence document for all the contacts with the same status. (Like one NOTIFY for all away clients, one for all busy, and so on...) Of course this case is not correct (related to RFCs) and really dirty but as it costs us nearly nothing, I thought that it's better to handle this with a loop.

* There was a mistake in the RPID processing, which was caused by xml
namespaces. This had the effect that the RPID status was never
actually considered to change the presence state in the contact list.
The visible extended state of other SC users in the contact list
(busy, away, on the
phone) was still adapted correctly, but this was just because of the
additional <note> element which should (as already stated in the
code) usually not be used to indicate detailed state information. The
main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any
namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained
in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS
being a wildcard "*" for an arbitrary namespace) for the RPID
handling. Then it doesn't matter to which namespace the element
belongs to.
  
You're certainly right, thanks for this catch and even more for your patch for it.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the
standard namespace for the common pidf tags, the current
implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?> <ns:presence
xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing
the calls to getElementsByTagName() by calls to
getElementsByTagNameNS(), using the namespace wildcard. However, I
did _not_ do this yet, mainly because I don't like these methods too
much anyway. This is because they do not only return immediate child
elements for the given node, but also elements in lower levels
(further nested in the document tree). Not sure if this is the ideal
handling, so I left this up to discussion. I'd prefer to use the
getChildNodes() method to get the immediate children for a node. What do you think?
  
Using the wildcard namespace is not really justified I think because, in this case, we are sure of the namespace we want, so yes, there is effectively a possible problem but we may find another solution.

I'd like to have your opinion on this before applying your patch.
Anyway thanks for all this work !

Cheers,
Ben

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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


#12

Really?
Can it be because of the format of the PIDF i´m passing to it?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:35
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Works fine, at least for me...

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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


#13

Might be - but that's better to judge if you just send an example... :wink:

Ralph Weires

···

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 4:37 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Really?
Can it be because of the format of the PIDF i´m passing to it?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:35
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Works fine, at least for me...

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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


#14

Hi Ben. I´m testing your patch and I would like to know wich classes and methods did you edit.
Thanks,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: terça-feira, 16 de Outubro de 2007 15:46
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again Ben,

I tried the changed version with some other clients and it looks good. The only of the clients whose presence now isn't recognised anymore is WengoPhone, but that's just due to the bad pidf implementation there - the documents of Wengo do not contain any xml namespace declaration, thus they don't meet the RFC requirements.

I think it's ok to be rather strict with the implementation - the most important thing is that the communication with clients behaving according to the RFCs works well.

Cheers,
Ralph

-----Original Message-----

From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr]

Sent: Tuesday, October 16, 2007 3:20 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Ralph, Joel, all,

I've commited a patch based on the Ralph's one.
Joel it should correct all your problems now. Wait for the next cruise control build and update.

Sadly we've got some problems with our sip provider for the moment so I'm not able to test deeply this patch, as I wanted to fix your problems quickly I've commited this anyway but know that this patch will perhaps need some corrections. If you still encounter difficulties with the new version, let us know please.

The patch consist in the Ralph's patch with the correction to handle every <contact> in a <tuple>, a correction to take into account the <note>s even if the document is not conform to the RFC and we now use the standard namespace to find PID informations.

I didn't added a fallback to a wildcard namespace for PID because I really dislike it. Using it with RPID was a necessity because it seems that many SIP clients aren't using the new namespace defined in the RFC recently. It's AFAIK not the case with the PIDF namespace so I prefer keep a strict conformance to RFC here until we find an exception to this. And even with an exception, it's theoretically the bad coded client which should be fixed.

Cheers,
Ben.

ralph.weires@hitec.lu a écrit :

Hi Ben,

- Concerning the loop for the contact elements: Ok, I didn't recognize
the reason for explicitely dealing with all eventually contained contact elements. You have a point there, right. I removed the loop just because it prevented me from doubling code parts in the method. No problem to leave that as it was, I guess :slight_smile: It's just that the additional handling of the information in <note>-tags (not desired by RFC, but sometimes used by clients as you mention in the code) currently only comes into play if there is at least one contact element. Perhaps it makes sense to put this part to the very beginning of the <tuple>-handling - it should give the same result for each contact element anyway. Additionally, by that way it can be used for the case of zero contact elements as well without duplicating the code.

- Regarding the general processing of xml namespaces: As long as the defined namespaces are more or less ignored, I think it would be good if SC would be able to accept the standard pidf tags also if they are not defined in the default namespace (i.e., with a prefix). You're right, we know in which namespace the tags are supposed to be, so we could use "urn:ietf:params:xml:ns:pidf" instead of the wildcard "*" in a getElementsByTagNameNS()-method to just get elements from the correct namespace. However, then we have a problem again in case that a pidf document doesn't contain correct namespace definitions at all...
Generally, a good solution IMHO would be a strict processing of the pidf documents by default (including the requirement for correct namespaces), with a fallback to some kind of "quirks" mode if any errors occur in the strict mode (similar to how several webbrowsers handle webpages).

Cheers,
Ralph

-----Original Message-----
From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr]
Sent: Monday, October 15, 2007 7:13 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP
presence implementation

Hi ralph,

I looked at all your propositions and at your patch. First, thank you very much for all the efforts you are spending in helping us to improve SC.

(more inline)

ralph.weires@hitec.lu a écrit :
  

Hi again,

I had some more problems with the SIMPLE (presence) functionality of
SC in cooperation with other SIMPLE clients, so I looked a little
closer into the presence implementation and found a few things to
enhance there. Attached is a patch for the
net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipIm
p l class which should correct some errors. Almost all performed
changes are contained in the setPidfPresenceStatus() method, which
handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the
resulting changes I made:
* I was not able to see the presence status of clients other than SC,
even though SC received NOTIFY messages containing the PIDF documents
with presence information of these clients. The reason for this is
the missing "contact" element in the tuples of the PIDF documents.
While the contact element SHOULD be included in tuples according to
RFC 3863, this is not mandatory. Even though I'd also say contact
should always be present, some other clients I used (X-Lite, pidgin)
do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the
value of the mandatory "entity" attribute of the root tag "presence"
is considered as contact address instead.
  
Yes, you're absolutely right, it's my bad. It seems that I just forget this case. Your patch correct it very well.

** There used to be a loop handling all contact elements in the
tuples (usually, there should be at most one). This is now handled as
with several other tags - if multiple contact elements should be
present for one tuple, only the last one is considered.
  
There was a special reason for this specific behavior so I'm not sure that your correction is a good idea here.
During the design, I thought that even if it's not allowed, some bad designed presence agent could one day send us a presence document for all the contacts with the same status. (Like one NOTIFY for all away clients, one for all busy, and so on...) Of course this case is not correct (related to RFCs) and really dirty but as it costs us nearly nothing, I thought that it's better to handle this with a loop.

* There was a mistake in the RPID processing, which was caused by xml
namespaces. This had the effect that the RPID status was never
actually considered to change the presence state in the contact list.
The visible extended state of other SC users in the contact list
(busy, away, on the
phone) was still adapted correctly, but this was just because of the
additional <note> element which should (as already stated in the
code) usually not be used to indicate detailed state information. The
main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any
namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained
in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS
being a wildcard "*" for an arbitrary namespace) for the RPID
handling. Then it doesn't matter to which namespace the element
belongs to.
  
You're certainly right, thanks for this catch and even more for your patch for it.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the
standard namespace for the common pidf tags, the current
implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?> <ns:presence
xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing
the calls to getElementsByTagName() by calls to
getElementsByTagNameNS(), using the namespace wildcard. However, I
did _not_ do this yet, mainly because I don't like these methods too
much anyway. This is because they do not only return immediate child
elements for the given node, but also elements in lower levels
(further nested in the document tree). Not sure if this is the ideal
handling, so I left this up to discussion. I'd prefer to use the
getChildNodes() method to get the immediate children for a node. What do you think?
  
Using the wildcard namespace is not really justified I think because, in this case, we are sure of the namespace we want, so yes, there is effectively a possible problem but we may find another solution.

I'd like to have your opinion on this before applying your patch.
Anyway thanks for all this work !

Cheers,
Ben

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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


#15

What type of example do you want?
a xml pidf?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:45
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Might be - but that's better to judge if you just send an example... :wink:

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 4:37 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Really?
Can it be because of the format of the PIDF i´m passing to it?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:35
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Works fine, at least for me...

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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


#16

Sure, that would be good (however I'm out of the office for today)

Anyway, perhaps we should consider doing this tomorrow in the IRC channel (see
http://www.sip-communicator.org/index.php/Development/MailingLists ), since I get more and more the feeling that we're starting to spam the mailing list...

Cheers,
Ralph Weires

···

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 4:47 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

What type of example do you want?
a xml pidf?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:45
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Might be - but that's better to judge if you just send an example... :wink:

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 4:37 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Really?
Can it be because of the format of the PIDF i´m passing to it?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:35
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Works fine, at least for me...

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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


#17

I attach an example of a Notify body that my sip-communicator receives from another user.
I don´t know why but when the method convertDocument is called the document is returned null.
Other thing that I dont understand is how the variable Document doc is manipulated inside this method.

BR,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

pidf.xml (1.65 KB)

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:45
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Might be - but that's better to judge if you just send an example... :wink:

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 4:37 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Really?
Can it be because of the format of the PIDF i´m passing to it?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:35
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Works fine, at least for me...

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

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


#18

Hi Joel,

For the moment, I only worked on the setPidfPresenceStatus method of the PresenceOpSet.

Note that if you want to follow the modifications done on the code, we have a wonderful mailing list for this. More info at :
http://www.sip-communicator.org/index.php/Development/MailingLists

Cheers,
Ben.

Joel Silva a �crit :

···

Hi Ben. I�m testing your patch and I would like to know wich classes and methods did you edit.
Thanks,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----
From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu] Sent: ter�a-feira, 16 de Outubro de 2007 15:46
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again Ben,

I tried the changed version with some other clients and it looks good. The only of the clients whose presence now isn't recognised anymore is WengoPhone, but that's just due to the bad pidf implementation there - the documents of Wengo do not contain any xml namespace declaration, thus they don't meet the RFC requirements.

I think it's ok to be rather strict with the implementation - the most important thing is that the communication with clients behaving according to the RFCs works well.

Cheers,
Ralph

-----Original Message-----
From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr]
Sent: Tuesday, October 16, 2007 3:20 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Ralph, Joel, all,

I've commited a patch based on the Ralph's one.
Joel it should correct all your problems now. Wait for the next cruise control build and update.

Sadly we've got some problems with our sip provider for the moment so I'm not able to test deeply this patch, as I wanted to fix your problems quickly I've commited this anyway but know that this patch will perhaps need some corrections. If you still encounter difficulties with the new version, let us know please.

The patch consist in the Ralph's patch with the correction to handle every <contact> in a <tuple>, a correction to take into account the <note>s even if the document is not conform to the RFC and we now use the standard namespace to find PID informations.

I didn't added a fallback to a wildcard namespace for PID because I really dislike it. Using it with RPID was a necessity because it seems that many SIP clients aren't using the new namespace defined in the RFC recently. It's AFAIK not the case with the PIDF namespace so I prefer keep a strict conformance to RFC here until we find an exception to this. And even with an exception, it's theoretically the bad coded client which should be fixed.

Cheers,
Ben.

ralph.weires@hitec.lu a �crit :
  

Hi Ben,

- Concerning the loop for the contact elements: Ok, I didn't recognize the reason for explicitely dealing with all eventually contained contact elements. You have a point there, right. I removed the loop just because it prevented me from doubling code parts in the method. No problem to leave that as it was, I guess :slight_smile: It's just that the additional handling of the information in <note>-tags (not desired by RFC, but sometimes used by clients as you mention in the code) currently only comes into play if there is at least one contact element. Perhaps it makes sense to put this part to the very beginning of the <tuple>-handling - it should give the same result for each contact element anyway. Additionally, by that way it can be used for the case of zero contact elements as well without duplicating the code.

- Regarding the general processing of xml namespaces: As long as the defined namespaces are more or less ignored, I think it would be good if SC would be able to accept the standard pidf tags also if they are not defined in the default namespace (i.e., with a prefix). You're right, we know in which namespace the tags are supposed to be, so we could use "urn:ietf:params:xml:ns:pidf" instead of the wildcard "*" in a getElementsByTagNameNS()-method to just get elements from the correct namespace. However, then we have a problem again in case that a pidf document doesn't contain correct namespace definitions at all...
Generally, a good solution IMHO would be a strict processing of the pidf documents by default (including the requirement for correct namespaces), with a fallback to some kind of "quirks" mode if any errors occur in the strict mode (similar to how several webbrowsers handle webpages).

Cheers,
Ralph

-----Original Message-----
From: Benoit Pradelle [mailto:ze_real_neo@yahoo.fr]
Sent: Monday, October 15, 2007 7:13 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi ralph,

I looked at all your propositions and at your patch. First, thank you very much for all the efforts you are spending in helping us to improve SC.

(more inline)

ralph.weires@hitec.lu a �crit :
  

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipIm
p l class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents.
While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence"
is considered as contact address instead.
  

Yes, you're absolutely right, it's my bad. It seems that I just forget this case. Your patch correct it very well.

** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.
  

There was a special reason for this specific behavior so I'm not sure that your correction is a good idea here.
During the design, I thought that even if it's not allowed, some bad designed presence agent could one day send us a presence document for all the contacts with the same status. (Like one NOTIFY for all away clients, one for all busy, and so on...) Of course this case is not correct (related to RFCs) and really dirty but as it costs us nearly nothing, I thought that it's better to handle this with a loop.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list.
The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the
code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.
  

You're certainly right, thanks for this catch and even more for your patch for it.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?> <ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the
getChildNodes() method to get the immediate children for a node. What do you think?
  

Using the wildcard namespace is not really justified I think because, in this case, we are sure of the namespace we want, so yes, there is effectively a possible problem but we may find another solution.

I'd like to have your opinion on this before applying your patch.
Anyway thanks for all this work !

Cheers,
Ben
    
###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

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


#19

Ok.
Thanks!

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

···

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:56
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Sure, that would be good (however I'm out of the office for today)

Anyway, perhaps we should consider doing this tomorrow in the IRC channel (see http://www.sip-communicator.org/index.php/Development/MailingLists ), since I get more and more the feeling that we're starting to spam the mailing list...

Cheers,
Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 4:47 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

What type of example do you want?
a xml pidf?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:45
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Might be - but that's better to judge if you just send an example... :wink:

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 4:37 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Really?
Can it be because of the format of the PIDF i´m passing to it?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 15:35
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Works fine, at least for me...

Ralph Weires

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----

From: Joel Silva [mailto:joel.silva@novabase.pt]

Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn´t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----

From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]

Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this:
<?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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


#20

Hi all,

Theoretically it's impossible that anything is written (so deleted) during the convertDocument execution as we are only handling StreamReader.
Is the xml correct ? If not, it's possible that the creation of the stream doesn't works and that the document used after is null.

Cheers,
Ben.

Joel Silva a �crit :

···

Ok.
Thanks!

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal
Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----
From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu] Sent: segunda-feira, 15 de Outubro de 2007 15:56
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Sure, that would be good (however I'm out of the office for today)

Anyway, perhaps we should consider doing this tomorrow in the IRC channel (see http://www.sip-communicator.org/index.php/Development/MailingLists ), since I get more and more the feeling that we're starting to spam the mailing list...

Cheers,
Ralph Weires

-----Original Message-----
From: Joel Silva [mailto:joel.silva@novabase.pt]
Sent: Monday, October 15, 2007 4:47 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

What type of example do you want?
a xml pidf?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----
From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]
Sent: segunda-feira, 15 de Outubro de 2007 15:45
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Might be - but that's better to judge if you just send an example... :wink:

Ralph Weires

-----Original Message-----
From: Joel Silva [mailto:joel.silva@novabase.pt]
Sent: Monday, October 15, 2007 4:37 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Really?
Can it be because of the format of the PIDF i�m passing to it?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----
From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]
Sent: segunda-feira, 15 de Outubro de 2007 15:35
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] Converted DOC == null

Works fine, at least for me...

Ralph Weires

-----Original Message-----
From: Joel Silva [mailto:joel.silva@novabase.pt]
Sent: Monday, October 15, 2007 3:47 PM
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] Converted DOC == null

Hi guys.
I found that in the setPidfPresenceStatus method when the method convertDocument(presenceDoc) its called it erases all the data of the presenceDoc.
Anyone have notice this? Or is just me?

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----
From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]
Sent: segunda-feira, 15 de Outubro de 2007 13:16
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Yes, that's exactly what the recent patch should already fix.

-----Original Message-----
From: Joel Silva [mailto:joel.silva@novabase.pt]
Sent: Monday, October 15, 2007 2:13 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

What is curious is that today I was able to publish my presence and clients other than SC was able to see my presence (although I do not see theirs) but other instances of SC still arent able to see my presence... Weird. I will keep looking for the problem.

Cheers,

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----
From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]
Sent: segunda-feira, 15 de Outubro de 2007 13:10
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi Joel,

I saw your problems from last week, too - however, I don't think this patch will do much to help with your problem. At least, I didn't have any problem with the presence functionality yet as long as the clients are just instances of Sip Communicator. The patch will mainly deal with presence problems between SC and _other_ clients.

Cheers,
Ralph

-----Original Message-----
From: Joel Silva [mailto:joel.silva@novabase.pt]
Sent: Monday, October 15, 2007 2:05 PM
To: dev@sip-communicator.dev.java.net
Subject: RE: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

I was just looking for the setPidfPresenceStatus method and saw that!
I didn�t understand very well what your patch do but I understood your idea.

Joel Silva
Analyst
.............................................................................................................
Novabase
Av. Eng. Duarte Pacheco, 15F . 1099-078 Lisboa - Portugal Tel. (+351) 213 836 300 . Fax (+351) 213 836 301 .
mailto:joel.silva@novabase.pt
www.novabase.pt

-----Original Message-----
From: ralph.weires@hitec.lu [mailto:ralph.weires@hitec.lu]
Sent: segunda-feira, 15 de Outubro de 2007 12:46
To: dev@sip-communicator.dev.java.net
Subject: [sip-comm-dev] [PATCH] pidf processing fix in SIP presence implementation

Hi again,

I had some more problems with the SIMPLE (presence) functionality of SC in cooperation with other SIMPLE clients, so I looked a little closer into the presence implementation and found a few things to enhance there. Attached is a patch for the net.java.sip.communicator.impl.protocol.sip.OperationSetPresenceSipImpl
class which should correct some errors. Almost all performed changes are contained in the setPidfPresenceStatus() method, which handles the PIDF documents of incoming NOTIFY requests.

Here is a detailed description of the problems I had and the resulting changes I made:
* I was not able to see the presence status of clients other than SC, even though SC received NOTIFY messages containing the PIDF documents with presence information of these clients. The reason for this is the missing "contact" element in the tuples of the PIDF documents. While the contact element SHOULD be included in tuples according to RFC 3863, this is not mandatory. Even though I'd also say contact should always be present, some other clients I used (X-Lite, pidgin) do not include this tag into their pidf documents. The changes I made to deal with this are:
** Accept PIDF tuples without contact element. In this case, the value of the mandatory "entity" attribute of the root tag "presence" is considered as contact address instead.
** There used to be a loop handling all contact elements in the tuples (usually, there should be at most one). This is now handled as with several other tags - if multiple contact elements should be present for one tuple, only the last one is considered.

* There was a mistake in the RPID processing, which was caused by xml namespaces. This had the effect that the RPID status was never actually considered to change the presence state in the contact list. The visible extended state of other SC users in the contact list (busy, away, on the
phone) was still adapted correctly, but this was just because of the additional <note> element which should (as already stated in the code) usually not be used to indicate detailed state information. The main problem here is the method
  parentnode.getElementsByTagName("childtag")
Using this method, only those "childtag" elements _without_ any namespace prefix are returned. So the call
  presence.getElementsByTagName(PERSON_ELEMENT);
returns an empty NodeList if there is an element dm:person contained in the pidf document. To deal with this, I changed these functions into
  parentnode.getElementsByTagName(ANY_NS, "childtag"); (with ANY_NS being a wildcard "*" for an arbitrary namespace) for the RPID handling. Then it doesn't matter to which namespace the element belongs to.

* The namespace problem described above actually applies to all calls to
  getElementsByTagName()
If a client e.g. chooses to create PIDF documents without using the standard namespace for the common pidf tags, the current implementation would not accept the document. Such as a document like this: <?xmlversion='1.0' encoding='UTF-8'?>
<ns:presence xmlns:ns='urn:ietf:params:xml:ns:pidf' ...
entity='sip:user@server'>
  <ns:tuple>
    <ns:status>
      <ns:basic>open</ns:basic>
    </ns:status>
  </ns:tuple>
</ns:presence>
This problem could be addressed the same way as above, by replacing the calls to getElementsByTagName() by calls to getElementsByTagNameNS(), using the namespace wildcard. However, I did _not_ do this yet, mainly because I don't like these methods too much anyway. This is because they do not only return immediate child elements for the given node, but also elements in lower levels (further nested in the document tree). Not sure if this is the ideal handling, so I left this up to discussion. I'd prefer to use the getChildNodes() method to get the immediate children for a node. What do you think?

Cheers,
Ralph Weires

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

###########################################
CONFIDENTIALITY: This e-mail and any attachments are confidential and may also be privileged.
If you are not the designated recipient, please notify the sender immediately by reply e-mail and destroy all copies (digital and paper).
Any unauthorized disclosure, distribution, copying, storage or use of the information contained in this e-mail or any attachments is strictly prohibited and may be unlawful.

---------------------------------------------------------------------
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

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