You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Aidan Skinner <ai...@apache.org> on 2009/03/31 10:36:55 UTC

Re: svn commit: r760305 - in /qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/jndi: ./ JNDIPropertyFileTest.java JNDITest.properties

On Tue, Mar 31, 2009 at 4:40 AM,  <ra...@apache.org> wrote:

> +    public void testQueueNamesWithTrailingSpaces() throws Exception
> +    {
> +        Queue queue = (Queue)ctx.lookup("QueueNameWithSpace");
> +        assertEquals("QueueNameWithSpace",queue.getQueueName());
> +    }

I missed the other commit with the actual calls to trim(), I'm not
sure this is the correct way to handle this case. The AMQP spec says:
"Queue names must have a length of between 1 and 255 characters
inclusive, must start with a digit, letter or underscores ('_')
character, and must be otherwise encoded in UTF-8." (AMQP 0-10, p226).

Your change adds an additional implementation rule that you cannot
have trailing spaces. Messages sent to 'ABC_' 'ABC__' and 'ABC' (s/_/
/g) all end up at the same place, and I'm really not sure that's
right.

- Aidan
-- 
Apache Qpid - World Domination through Advanced Message Queueing
http://qpid.apache.org

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: svn commit: r760305 - in /qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/jndi: ./ JNDIPropertyFileTest.java JNDITest.properties

Posted by Gordon Sim <gs...@redhat.com>.
Aidan Skinner wrote:
> On Tue, Mar 31, 2009 at 10:19 AM, Gordon Sim <gs...@redhat.com> wrote:
> 
>> If there is a desire to allow names with trailing spaces to be defined in
>> the properties file in my view that should require some more explicit
>> notation e.g. quotes around the name.
> 
> That's fair. I'm not sure how to enable people to do this though.
> 
> More post-processing to something that's fairly standard seems odd and
> un-Javay, so things like quotes and escape strings seems unlikely to
> work. Having flags to toggle how that's processed also seems bad.
> 
> Perhaps the best way is to always trim and if you need trailing spaces
> you need to create them some other way. That seems a little lame, but
> it's quite a niche concern and trimming is probably what 8 out of 10
> cats prefer.

I think thats a good approach. I very much doubt anyone needs to have 
trailing spaces in their queue name and in any unusual case where that 
is required, they will have to use a config mechanism other than the 
standard properties file format.


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: svn commit: r760305 - in /qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/jndi: ./ JNDIPropertyFileTest.java JNDITest.properties

Posted by Aidan Skinner <ai...@apache.org>.
On Tue, Mar 31, 2009 at 10:19 AM, Gordon Sim <gs...@redhat.com> wrote:

> If there is a desire to allow names with trailing spaces to be defined in
> the properties file in my view that should require some more explicit
> notation e.g. quotes around the name.

That's fair. I'm not sure how to enable people to do this though.

More post-processing to something that's fairly standard seems odd and
un-Javay, so things like quotes and escape strings seems unlikely to
work. Having flags to toggle how that's processed also seems bad.

Perhaps the best way is to always trim and if you need trailing spaces
you need to create them some other way. That seems a little lame, but
it's quite a niche concern and trimming is probably what 8 out of 10
cats prefer.

- Aidan
-- 
Apache Qpid - World Domination through Advanced Message Queueing
http://qpid.apache.org

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: svn commit: r760305 - in /qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/jndi: ./ JNDIPropertyFileTest.java JNDITest.properties

Posted by Gordon Sim <gs...@redhat.com>.
Aidan Skinner wrote:
> On Tue, Mar 31, 2009 at 4:40 AM,  <ra...@apache.org> wrote:
> 
>> +    public void testQueueNamesWithTrailingSpaces() throws Exception
>> +    {
>> +        Queue queue = (Queue)ctx.lookup("QueueNameWithSpace");
>> +        assertEquals("QueueNameWithSpace",queue.getQueueName());
>> +    }
> 
> I missed the other commit with the actual calls to trim(), I'm not
> sure this is the correct way to handle this case. The AMQP spec says:
> "Queue names must have a length of between 1 and 255 characters
> inclusive, must start with a digit, letter or underscores ('_')
> character, and must be otherwise encoded in UTF-8." (AMQP 0-10, p226).
> 
> Your change adds an additional implementation rule that you cannot
> have trailing spaces. Messages sent to 'ABC_' 'ABC__' and 'ABC' (s/_/
> /g) all end up at the same place, and I'm really not sure that's
> right.

What AMQP allows for queue names and what is sensible when interpreting 
a properties file are two separate things in my view.

I personally think that by default trailing whitespace in the properties 
file entry for a particular Destination _should_ be ignored.

If there is a desire to allow names with trailing spaces to be defined 
in the properties file in my view that should require some more explicit 
notation e.g. quotes around the name.

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org