Bug 509454 - [RFE] Add validation for the '--cluster-url' qpidd option
Summary: [RFE] Add validation for the '--cluster-url' qpidd option
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: qpid-cpp
Version: 1.1.2
Hardware: All
OS: Linux
medium
medium
Target Milestone: 1.3
: ---
Assignee: Gordon Sim
QA Contact: MRG Quality Engineering
URL:
Whiteboard: This is a TSX requested feature that ...
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-07-02 21:03 UTC by Frank Hirtz
Modified: 2018-11-14 19:57 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
The current Advanced Message Queuing Protocol (AMQP) specification allows a cluster URL in the following form: [host [":" port] ] However, this also allows URLs such as "amqp:/amqp:tcp:", which can be confusing. With this update, a hostname is required, so that the valid cluster URL now looks as follows: [[amqp:]tcp:]hostname[:port] This change ensures that all URLs now take the reasonable form.
Clone Of:
Environment:
Last Closed: 2010-10-14 16:11:51 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2010:0773 0 normal SHIPPED_LIVE Moderate: Red Hat Enterprise MRG Messaging and Grid Version 1.3 2010-10-14 15:56:44 UTC

Description Frank Hirtz 2009-07-02 21:03:22 UTC
In testing, it was noted that there was no validation on the --cluster-url qpidd option. We would like to see validation added to catch bad inputs there.

<snip>
$ pgrep qpidd -fl
5475 /usr/sbin/qpidd --daemon --port=5673 --data-dir=/app/qgw_ed2/ryan/common/data/3 --pid-dir=/app/qgw_ed2/ryan/common/pid --worker-threads=4 --tcp-nodelay --auth=no --no-module-dir --default-queue-limit=0 --wait=60 --log-enable=notice+ --log-to-file=/app/qgw_ed2/ryan/common/log/qpidd_2009-07-02.log --log-to-syslog=no --load-module=/usr/lib64/qpid/daemon/cluster.so --cluster-name=qgwed2_5673 --cluster-url=amqp:tcp::5673
</snip>

Comment 2 ppecka 2010-03-22 15:33:52 UTC
1)
 when qpidd started with "--cluster-url=amqp::22". qpid-cluster shows following:
 # qpid-cluster
       Members: ID=10.43.54.1:24749 URL=amqp:tcp:127.0.0.1:22


From AMQP 0-10 documentation:
Domain: connection.amqp-host-url
amqp_url = "amqp:" prot_addr_list
prot_addr_list = [prot_addr ","]* prot_addr
prot_addr = tcp_prot_addr | tls_prot_addr
tcp_prot_addr = tcp_id tcp_addr
tcp_id = "tcp:" | ""
tcp_addr = [host [":" port] ]
host = <as per http://www.ietf.org/rfc/rfc3986.txt>
port = number


to see it working that way, AMQP should read something similar to tcp_addr = [host [":" port] ] | [":port"] | ""
am i right?


2)
having in mind chapter 6.2.3 of http://www.ietf.org/rfc/rfc3986.txt, i'm not able to find in AMQP specification what should happend if host is unspecified, especially that host part of URI should be filled with localhost address.

Comment 3 Gordon Sim 2010-04-13 16:36:47 UTC
I believe the fix to require the hostname is simple:

Index: src/qpid/Url.cpp
===================================================================
--- src/qpid/Url.cpp	(revision 933532)
+++ src/qpid/Url.cpp	(working copy)
@@ -116,7 +116,7 @@
         const char* start=i;
         while (unreserved() || pctEncoded())
             ;
-        if (start == i) h = LOCALHOST; // Default
+        if (start == i) return false;//host is required
         else h.assign(start, i);
         return true;
     }

So unless the above is intended, I suggest we commit that and close the issue.

Comment 4 Gordon Sim 2010-04-14 14:33:05 UTC
Fixed in r933970. The host is now required.

Comment 5 ppecka 2010-04-15 18:44:46 UTC
when cluster-url will be set to "amqp:" or "amqp:tcp:" without host and/or port will it proceed?

Comment 6 Gordon Sim 2010-04-15 18:57:17 UTC
If the cluster-url is not valid qpidd won't start, so host must be included (port neede not be).

Comment 9 ppecka 2010-07-02 11:25:50 UTC
ok i've generated over 131072 URL's, and checked with 
echo "${cluster_urls}" | grep -Poe '^((amqp:)*(tcp:)*)*([\d|\w|\-\.]+)(:([1-5]*[0-9]{0,4}|((6553[0-6])|(655[0-2][0-9])|(65[0-4][0-9][0-9])|(6[0-4][0-9]{3}))))*$' | grep -v -Poe '(^:+)|((:+$)|(:0+$))'

current rule for cluster-url seems to be: [[amqp:]tcp:]hostname[:port]

if only hostname is set or any other subset, cluster url is supplemented with defaults so for cluster-url=my-host-name its transformed into form of amqp:tcp:my-host-name:5672

so i guess we need to change documentation for MRG or AMQP.

--> VERIFIED

Comment 10 mick 2010-10-07 16:48:34 UTC
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Cause:
    The current AMQP spec says, for connection.amqp-host-url
        tcp_addr = [host [":" port] ]

  Consequence:
    This allows nonsense cluster URLs such as:
        amqp:/amqp:tcp:
    which are confusing to customers.

  
  Fix:
    require a hostname


  Result:
    current cluster URL rule is now:
      [[amqp:]tcp:]hostname[:port]
    which generates more reasonable URLs.

Comment 11 Jaromir Hradilek 2010-10-07 18:20:23 UTC
    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1,18 +1,9 @@
-Cause:
-    The current AMQP spec says, for connection.amqp-host-url
-        tcp_addr = [host [":" port] ]
+The current Advanced Message Queuing Protocol (AMQP) specification allows a cluster URL in the following form:
 
-  Consequence:
-    This allows nonsense cluster URLs such as:
-        amqp:/amqp:tcp:
-    which are confusing to customers.
+  [host [":" port] ]
 
-  
-  Fix:
-    require a hostname
+However, this also allows URLs such as "amqp:/amqp:tcp:", which can be confusing. With this update, a hostname is required, so that the valid cluster URL now looks as follows:
 
-
+  [[amqp:]tcp:]hostname[:port]
-  Result:
+  
-    current cluster URL rule is now:
+This change ensures that all URLs now take the reasonable form.-      [[amqp:]tcp:]hostname[:port]
-    which generates more reasonable URLs.

Comment 13 errata-xmlrpc 2010-10-14 16:11:51 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2010-0773.html


Note You need to log in before you can comment on or make changes to this bug.