Bug 509454 - [RFE] Add validation for the '--cluster-url' qpidd option
[RFE] Add validation for the '--cluster-url' qpidd option
Status: CLOSED ERRATA
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: qpid-cpp (Show other bugs)
1.1.2
All Linux
medium Severity medium
: 1.3
: ---
Assigned To: Gordon Sim
MRG Quality Engineering
This is a TSX requested feature that ...
: FutureFeature
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-02 17:03 EDT by Frank Hirtz
Modified: 2010-10-14 12:11 EDT (History)
6 users (show)

See Also:
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.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-10-14 12:11:51 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Frank Hirtz 2009-07-02 17:03:22 EDT
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 11:33:52 EDT
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 12:36:47 EDT
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 10:33:05 EDT
Fixed in r933970. The host is now required.
Comment 5 ppecka 2010-04-15 14:44:46 EDT
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 14:57:17 EDT
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 07:25:50 EDT
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 12:48:34 EDT
    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 14:20:23 EDT
    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 12:11:51 EDT
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.