You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Bernd Fondermann <bf...@brainlounge.de> on 2009/07/07 11:21:18 UTC

Re: svn commit: r791261 - /mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java

mjakl@apache.org wrote:
> Author: mjakl
> Date: Sun Jul  5 15:42:12 2009
> New Revision: 791261
> 
> URL: http://svn.apache.org/viewvc?rev=791261&view=rev
> Log:
> Don't trust the sender with the "to" address, use the one in the sessionContext.

Now _you_ are getting overly defensive. ;-)

If my mind serves me right, the to address has been checked before (in
ProtocolWorker?), so it's save to use it. And I don't think
getServerJID() will neccessarily give you the right one in every case
either.

  Bernd


> 
> Modified:
>     mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java
> 
> Modified: mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java
> URL: http://svn.apache.org/viewvc/mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java?rev=791261&r1=791260&r2=791261&view=diff
> ==============================================================================
> --- mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java (original)
> +++ mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java Sun Jul  5 15:42:12 2009
> @@ -66,7 +66,7 @@
>              ServerRuntimeContext serverRuntimeContext,
>              SessionContext sessionContext) {
>          Entity sender = getFromAddress(stanza, sessionContext);
> -        Entity receiver = stanza.getTo();
> +        Entity receiver = sessionContext.getServerJID();
>  
>          String iqStanzaID = stanza.getAttributeValue("id");
>  
> 
> 
> 


Re: svn commit: r791261 - /mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java

Posted by Bernd Fondermann <bf...@brainlounge.de>.
Michael Jakl wrote:
> Hi!
> 
> On Tue, Jul 7, 2009 at 12:06, Bernd Fondermann<bf...@brainlounge.de> wrote:
>> Michael Jakl wrote:
>>> Currently it should (why not?), but as soon as we allow components to
>>> have different domains it doesn't anymore.
>> That's what I had in mind. What if the pubsub is addressed as
>> pubsub@domain.org, wouldn't this trigger a problem already?
> 
> Very likely. I was planning to address pubsub nodes as domain.tld with a node
> attribute. This is also conform to the disco requests.
> 
> The spec calls pubsub nodes addressable as user@domain.tld "virtual". To
> support addressing using this schema (without big changes), we'd have to
> introduce a virtual user acting as the pubsub service... .
> 
>>> Anyway, the JID for the module should be configured elsewhere, and not
>>> take from the request. I'll fix that too tonight.
>> +1.
>>
>> In a related thought, what do you by the way think about introducing a
>> PubSubConfiguration class where all of the many options possible in
>> pubsub can be collected? The module JID could be one of them.
> 
> The pubsub module doesn't have that many configuration options as far as I can
> see. 

When I read the spec, I just thought at multiple places: Oh, that should
go into a config option! But I never kept a list of those, unfortunately.

> Most of the options are related to LeafNodes, but I'll keep it in mind.
> 
> Concerning the options (together with disco): Somehow I'd like to separate the
> collection of the possible options out of the disco-part (currently all
> supported features have to be put into one class). Something like going through
> all handlers and collection whatever features they support would make many
> things much easier. I think this is exactly what you do with the handlers
> (which handler is responsible for which kind of request).  But that's more or
> less future talk.

+1

> 
> Michael


Re: svn commit: r791261 - /mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java

Posted by Michael Jakl <ja...@gmail.com>.
Hi!

On Tue, Jul 7, 2009 at 12:06, Bernd Fondermann<bf...@brainlounge.de> wrote:
> Michael Jakl wrote:
>> Currently it should (why not?), but as soon as we allow components to
>> have different domains it doesn't anymore.
>
> That's what I had in mind. What if the pubsub is addressed as
> pubsub@domain.org, wouldn't this trigger a problem already?

Very likely. I was planning to address pubsub nodes as domain.tld with a node
attribute. This is also conform to the disco requests.

The spec calls pubsub nodes addressable as user@domain.tld "virtual". To
support addressing using this schema (without big changes), we'd have to
introduce a virtual user acting as the pubsub service... .

>> Anyway, the JID for the module should be configured elsewhere, and not
>> take from the request. I'll fix that too tonight.
>
> +1.
>
> In a related thought, what do you by the way think about introducing a
> PubSubConfiguration class where all of the many options possible in
> pubsub can be collected? The module JID could be one of them.

The pubsub module doesn't have that many configuration options as far as I can
see. Most of the options are related to LeafNodes, but I'll keep it in mind.

Concerning the options (together with disco): Somehow I'd like to separate the
collection of the possible options out of the disco-part (currently all
supported features have to be put into one class). Something like going through
all handlers and collection whatever features they support would make many
things much easier. I think this is exactly what you do with the handlers
(which handler is responsible for which kind of request).  But that's more or
less future talk.

Michael

Re: svn commit: r791261 - /mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java

Posted by Bernd Fondermann <bf...@brainlounge.de>.
Michael Jakl wrote:
> Hi!
> 
> On Tue, Jul 7, 2009 at 11:21, Bernd Fondermann<bf...@brainlounge.de> wrote:
>>> Don't trust the sender with the "to" address, use the one in the sessionContext.
>> Now _you_ are getting overly defensive. ;-)
>>
>> If my mind serves me right, the to address has been checked before (in
>> ProtocolWorker?), so it's save to use it. And I don't think
>> getServerJID() will neccessarily give you the right one in every case
>> either.
> 
> Currently it should (why not?), but as soon as we allow components to
> have different domains it doesn't anymore.

That's what I had in mind. What if the pubsub is addressed as
pubsub@domain.org, wouldn't this trigger a problem already?

> Anyway, the JID for the module should be configured elsewhere, and not
> take from the request. I'll fix that too tonight.

+1.

In a related thought, what do you by the way think about introducing a
PubSubConfiguration class where all of the many options possible in
pubsub can be collected? The module JID could be one of them.

  Bernd


Re: svn commit: r791261 - /mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/handler/PubSubCreateNodeHandler.java

Posted by Michael Jakl <ja...@gmail.com>.
Hi!

On Tue, Jul 7, 2009 at 11:21, Bernd Fondermann<bf...@brainlounge.de> wrote:
>> Don't trust the sender with the "to" address, use the one in the sessionContext.
>
> Now _you_ are getting overly defensive. ;-)
>
> If my mind serves me right, the to address has been checked before (in
> ProtocolWorker?), so it's save to use it. And I don't think
> getServerJID() will neccessarily give you the right one in every case
> either.

Currently it should (why not?), but as soon as we allow components to
have different domains it doesn't anymore.

Anyway, the JID for the module should be configured elsewhere, and not
take from the request. I'll fix that too tonight.

Michael