You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2015/11/19 22:45:13 UTC
qpid-proton git commit: PROTON-995: Fix pn_url_str to use URL hex
escapes to generate a valid URL string.
Repository: qpid-proton
Updated Branches:
refs/heads/master cd8ad96f2 -> aa226be31
PROTON-995: Fix pn_url_str to use URL hex escapes to generate a valid URL string.
pn_url_parse decodes URL encoded hexadecimal escape sequences in the username
and password, but pn_url_str was not re-encoding them resulting in an invalid
URL string if they contained the forbidden characters ':', '/' or '@'.
This fixes pn_url_str to hex-encode those characters in username and password fields.
PROTON-995 was reported against the python binding, the python Url class wraps
a pn_url_t so this also fixes the python problem.
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/aa226be3
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/aa226be3
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/aa226be3
Branch: refs/heads/master
Commit: aa226be319135943fd1a7545d030ff3324ab8cc9
Parents: cd8ad96
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Nov 19 12:36:53 2015 -0500
Committer: Alan Conway <ac...@redhat.com>
Committed: Thu Nov 19 15:22:44 2015 -0500
----------------------------------------------------------------------
proton-c/include/proton/url.h | 12 +++
proton-c/src/tests/parse-url.c | 150 +++++++++++++++++-------------
proton-c/src/url.c | 24 ++++-
proton-c/src/util.c | 7 --
tests/python/proton_tests/interop.py | 2 +-
5 files changed, 118 insertions(+), 77 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/aa226be3/proton-c/include/proton/url.h
----------------------------------------------------------------------
diff --git a/proton-c/include/proton/url.h b/proton-c/include/proton/url.h
index 9731f56..01e1977 100644
--- a/proton-c/include/proton/url.h
+++ b/proton-c/include/proton/url.h
@@ -40,6 +40,18 @@ typedef struct pn_url_t pn_url_t;
PN_EXTERN pn_url_t *pn_url(void);
/** Parse a string URL as a pn_url_t.
+ *
+ * URL syntax:
+ * [ <scheme> :// ] [ <user> [ : <password> ] @ ] <host> [ : <port> ] [ / <path> ]
+ *
+ * <scheme>, <user>, <password>, <port> cannot contain any of '@', ':', '/'
+ *
+ * If the first character of <host> is '[' then it can contain any character up
+ * to ']' (this is to allow IPv6 literal syntax). Otherwise it also cannot
+ * contain '@', ':', '/'
+ *
+ * <path> can contain any character
+ *
*@param[in] url A URL string.
*@return The parsed pn_url_t or NULL if url is not a valid URL string.
*/
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/aa226be3/proton-c/src/tests/parse-url.c
----------------------------------------------------------------------
diff --git a/proton-c/src/tests/parse-url.c b/proton-c/src/tests/parse-url.c
index 829e9ab..c17ff1f 100644
--- a/proton-c/src/tests/parse-url.c
+++ b/proton-c/src/tests/parse-url.c
@@ -24,84 +24,100 @@
#include <stdlib.h>
#include <string.h>
-// No point in running this code if assert doesn't work!
-#undef NDEBUG
-#include <assert.h>
-
#include "proton/type_compat.h"
#include "proton/error.h"
-#include "util.h"
+#include "proton/url.h"
+
+static bool verify(const char* what, const char* want, const char* got) {
+ bool eq = (want == got || (want && got && strcmp(want, got) == 0));
+ if (!eq) printf(" %s: '%s' != '%s'\n", what, want, got);
+ return eq;
+}
+
+static bool test(const char* url, const char* scheme, const char* user, const char* pass, const char* host, const char* port, const char*path)
+{
+ pn_url_t *purl = pn_url_parse(url);
+ bool ok =
+ verify("scheme", scheme, pn_url_get_scheme(purl)) &&
+ verify("user", user, pn_url_get_username(purl)) &&
+ verify("pass", pass, pn_url_get_password(purl)) &&
+ verify("host", host, pn_url_get_host(purl)) &&
+ verify("port", port, pn_url_get_port(purl)) &&
+ verify("path", path, pn_url_get_path(purl));
+ pn_url_free(purl);
+ return ok;
+}
-static inline bool equalStrP(const char* s1, const char* s2)
+// Run test and additionally verify the round trip of parse and stringify
+// matches original string.
+static bool testrt(const char* url, const char* scheme, const char* user, const char* pass, const char* host, const char* port, const char*path)
{
- return (s1==s2) || (s1 && s2 && strcmp(s1,s2)==0);
+ bool ok = test(url, scheme, user, pass, host, port, path);
+ pn_url_t *purl = pn_url_parse(url);
+ ok = ok && verify("url", url, pn_url_str(purl));
+ pn_url_free(purl);
+ return ok;
}
-static bool test_url_parse(const char* url0, const char* scheme0, const char* user0, const char* pass0, const char* host0, const char* port0, const char*path0)
+#define TEST(EXPR) \
+ do { if (!(EXPR)) { printf("%s:%d: %s\n\n", __FILE__, __LINE__, #EXPR); failed++; } } while(0)
+
+int main(int argc, char **argv)
{
- char* url = (char*)malloc(strlen(url0)+1);
- char* scheme = 0;
- char* user = 0;
- char* pass = 0;
- char* host = 0;
- char* port = 0;
- char* path = 0;
+ int failed = 0;
+ TEST(testrt("/Foo.bar:90087@somewhere", 0, 0, 0, 0, 0, "Foo.bar:90087@somewhere"));
+ TEST(testrt("host", 0, 0, 0, "host", 0, 0));
+ TEST(testrt("host:423", 0, 0, 0, "host", "423", 0));
+ TEST(testrt("user@host", 0, "user", 0, "host", 0, 0));
- strcpy(url, url0);
+ // Can't round-trip passwords with ':', not strictly legal but the parser allows it.
+ TEST(test("user:1243^&^:pw@host:423", 0, "user", "1243^&^:pw", "host", "423", 0));
+ TEST(test("user:1243^&^:pw@host:423/Foo.bar:90087", 0, "user", "1243^&^:pw", "host", "423", "Foo.bar:90087"));
+ TEST(test("user:1243^&^:pw@host:423/Foo.bar:90087@somewhere", 0, "user", "1243^&^:pw", "host", "423", "Foo.bar:90087@somewhere"));
- pni_parse_url(url, &scheme, &user, &pass, &host, &port, &path);
- bool r = equalStrP(scheme,scheme0) &&
- equalStrP(user, user0) &&
- equalStrP(pass, pass0) &&
- equalStrP(host, host0) &&
- equalStrP(port, port0) &&
- equalStrP(path, path0);
+ TEST(testrt("[::1]", 0, 0, 0, "::1", 0, 0));
+ TEST(testrt("[::1]:amqp", 0, 0, 0, "::1", "amqp", 0));
+ TEST(testrt("user@[::1]", 0, "user", 0, "::1", 0, 0));
+ TEST(testrt("user@[::1]:amqp", 0, "user", 0, "::1", "amqp", 0));
- free(url);
+ // Can't round-trip passwords with ':', not strictly legal but the parser allows it.
+ TEST(test("user:1243^&^:pw@[::1]:amqp", 0, "user", "1243^&^:pw", "::1", "amqp", 0));
+ TEST(test("user:1243^&^:pw@[::1]:amqp/Foo.bar:90087", 0, "user", "1243^&^:pw", "::1", "amqp", "Foo.bar:90087"));
+ TEST(test("user:1243^&^:pw@[::1:amqp/Foo.bar:90087", 0, "user", "1243^&^:pw", "[", ":1:amqp", "Foo.bar:90087"));
+ TEST(test("user:1243^&^:pw@::1]:amqp/Foo.bar:90087", 0, "user", "1243^&^:pw", 0, ":1]:amqp", "Foo.bar:90087"));
- return r;
-}
+ TEST(testrt("amqp://user@[::1]", "amqp", "user", 0, "::1", 0, 0));
+ TEST(testrt("amqp://user@[::1]:amqp", "amqp", "user", 0, "::1", "amqp", 0));
+ TEST(testrt("amqp://user@[1234:52:0:1260:f2de:f1ff:fe59:8f87]:amqp", "amqp", "user", 0, "1234:52:0:1260:f2de:f1ff:fe59:8f87", "amqp", 0));
-int main(int argc, char **argv)
-{
- assert(test_url_parse("", 0, 0, 0, "", 0, 0));
- assert(test_url_parse("/Foo.bar:90087@somewhere", 0, 0, 0, "", 0, "Foo.bar:90087@somewhere"));
- assert(test_url_parse("host", 0, 0, 0, "host", 0, 0));
- assert(test_url_parse("host:423", 0, 0, 0, "host", "423", 0));
- assert(test_url_parse("user@host", 0, "user", 0, "host", 0, 0));
- assert(test_url_parse("user:1243^&^:pw@host:423", 0, "user", "1243^&^:pw", "host", "423", 0));
- assert(test_url_parse("user:1243^&^:pw@host:423/Foo.bar:90087", 0, "user", "1243^&^:pw", "host", "423", "Foo.bar:90087"));
- assert(test_url_parse("user:1243^&^:pw@host:423/Foo.bar:90087@somewhere", 0, "user", "1243^&^:pw", "host", "423", "Foo.bar:90087@somewhere"));
- assert(test_url_parse("[::1]", 0, 0, 0, "::1", 0, 0));
- assert(test_url_parse("[::1]:amqp", 0, 0, 0, "::1", "amqp", 0));
- assert(test_url_parse("user@[::1]", 0, "user", 0, "::1", 0, 0));
- assert(test_url_parse("user@[::1]:amqp", 0, "user", 0, "::1", "amqp", 0));
- assert(test_url_parse("user:1243^&^:pw@[::1]:amqp", 0, "user", "1243^&^:pw", "::1", "amqp", 0));
- assert(test_url_parse("user:1243^&^:pw@[::1]:amqp/Foo.bar:90087", 0, "user", "1243^&^:pw", "::1", "amqp", "Foo.bar:90087"));
- assert(test_url_parse("user:1243^&^:pw@[::1:amqp/Foo.bar:90087", 0, "user", "1243^&^:pw", "[", ":1:amqp", "Foo.bar:90087"));
- assert(test_url_parse("user:1243^&^:pw@::1]:amqp/Foo.bar:90087", 0, "user", "1243^&^:pw", "", ":1]:amqp", "Foo.bar:90087"));
- assert(test_url_parse("amqp://user@[::1]", "amqp", "user", 0, "::1", 0, 0));
- assert(test_url_parse("amqp://user@[::1]:amqp", "amqp", "user", 0, "::1", "amqp", 0));
- assert(test_url_parse("amqp://user@[1234:52:0:1260:f2de:f1ff:fe59:8f87]:amqp", "amqp", "user", 0, "1234:52:0:1260:f2de:f1ff:fe59:8f87", "amqp", 0));
- assert(test_url_parse("amqp://user:1243^&^:pw@[::1]:amqp", "amqp", "user", "1243^&^:pw", "::1", "amqp", 0));
- assert(test_url_parse("amqp://user:1243^&^:pw@[::1]:amqp/Foo.bar:90087", "amqp", "user", "1243^&^:pw", "::1", "amqp", "Foo.bar:90087"));
- assert(test_url_parse("amqp://host", "amqp", 0, 0, "host", 0, 0));
- assert(test_url_parse("amqp://user@host", "amqp", "user", 0, "host", 0, 0));
- assert(test_url_parse("amqp://user@host/path:%", "amqp", "user", 0, "host", 0, "path:%"));
- assert(test_url_parse("amqp://user@host:5674/path:%", "amqp", "user", 0, "host", "5674", "path:%"));
- assert(test_url_parse("amqp://user@host/path:%", "amqp", "user", 0, "host", 0, "path:%"));
- assert(test_url_parse("amqp://bigbird@host/queue@host", "amqp", "bigbird", 0, "host", 0, "queue@host"));
- assert(test_url_parse("amqp://host/queue@host", "amqp", 0, 0, "host", 0, "queue@host"));
- assert(test_url_parse("amqp://host:9765/queue@host", "amqp", 0, 0, "host", "9765", "queue@host"));
- assert(test_url_parse("user:pass%2fword@host", 0, "user", "pass/word", "host", 0, 0));
- assert(test_url_parse("user:pass%2Fword@host", 0, "user", "pass/word", "host", 0, 0));
- assert(test_url_parse("us%2fer:password@host", 0, "us/er", "password", "host", 0, 0));
- assert(test_url_parse("us%2Fer:password@host", 0, "us/er", "password", "host", 0, 0));
- assert(test_url_parse("user:pass%2fword%@host", 0, "user", "pass/word%", "host", 0, 0));
- assert(test_url_parse("localhost/temp-queue://ID:ganymede-36663-1408448359876-2:123:0", 0, 0, 0, "localhost", 0, "temp-queue://ID:ganymede-36663-1408448359876-2:123:0"));
- assert(test_url_parse("/temp-queue://ID:ganymede-36663-1408448359876-2:123:0", 0, 0, 0, "", 0, "temp-queue://ID:ganymede-36663-1408448359876-2:123:0"));
- assert(test_url_parse("amqp://localhost/temp-queue://ID:ganymede-36663-1408448359876-2:123:0", "amqp", 0, 0, "localhost", 0, "temp-queue://ID:ganymede-36663-1408448359876-2:123:0"));
+ // Can't round-trip passwords with ':', not strictly legal but the parser allows it.
+ TEST(test("amqp://user:1243^&^:pw@[::1]:amqp", "amqp", "user", "1243^&^:pw", "::1", "amqp", 0));
+ TEST(test("amqp://user:1243^&^:pw@[::1]:amqp/Foo.bar:90087", "amqp", "user", "1243^&^:pw", "::1", "amqp", "Foo.bar:90087"));
+
+ TEST(testrt("amqp://host", "amqp", 0, 0, "host", 0, 0));
+ TEST(testrt("amqp://user@host", "amqp", "user", 0, "host", 0, 0));
+ TEST(testrt("amqp://user@host/path:%", "amqp", "user", 0, "host", 0, "path:%"));
+ TEST(testrt("amqp://user@host:5674/path:%", "amqp", "user", 0, "host", "5674", "path:%"));
+ TEST(testrt("amqp://user@host/path:%", "amqp", "user", 0, "host", 0, "path:%"));
+ TEST(testrt("amqp://bigbird@host/queue@host", "amqp", "bigbird", 0, "host", 0, "queue@host"));
+ TEST(testrt("amqp://host/queue@host", "amqp", 0, 0, "host", 0, "queue@host"));
+ TEST(testrt("amqp://host:9765/queue@host", "amqp", 0, 0, "host", "9765", "queue@host"));
+ TEST(test("user:pass%2fword@host", 0, "user", "pass/word", "host", 0, 0));
+ TEST(testrt("user:pass%2Fword@host", 0, "user", "pass/word", "host", 0, 0));
+ // Can't round-trip passwords with lowercase hex encoding
+ TEST(test("us%2fer:password@host", 0, "us/er", "password", "host", 0, 0));
+ TEST(testrt("us%2Fer:password@host", 0, "us/er", "password", "host", 0, 0));
+ // Can't round-trip passwords with lowercase hex encoding
+ TEST(test("user:pass%2fword%@host", 0, "user", "pass/word%", "host", 0, 0));
+ TEST(testrt("localhost/temp-queue://ID:ganymede-36663-1408448359876-2:123:0", 0, 0, 0, "localhost", 0, "temp-queue://ID:ganymede-36663-1408448359876-2:123:0"));
+ TEST(testrt("/temp-queue://ID:ganymede-36663-1408448359876-2:123:0", 0, 0, 0, 0, 0, "temp-queue://ID:ganymede-36663-1408448359876-2:123:0"));
+ TEST(testrt("amqp://localhost/temp-queue://ID:ganymede-36663-1408448359876-2:123:0", "amqp", 0, 0, "localhost", 0, "temp-queue://ID:ganymede-36663-1408448359876-2:123:0"));
+ // PROTON-995
+ TEST(testrt("amqps://%40user%2F%3A:%40pass%2F%3A@example.net/some_topic",
+ "amqps", "@user/:", "@pass/:", "example.net", 0, "some_topic"));
+ TEST(testrt("amqps://user%2F%3A=:pass%2F%3A=@example.net/some_topic",
+ "amqps", "user/:=", "pass/:=", "example.net", 0, "some_topic"));
// Really perverse url
- assert(test_url_parse("://:@://:", "", "", "", "", "", "/:"));
- return 0;
+ TEST(testrt("://:@://:", "", "", "", 0, "", "/:"));
+ return failed;
}
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/aa226be3/proton-c/src/url.c
----------------------------------------------------------------------
diff --git a/proton-c/src/url.c b/proton-c/src/url.c
index 2e1c4f0..1e70f19 100644
--- a/proton-c/src/url.c
+++ b/proton-c/src/url.c
@@ -130,13 +130,33 @@ PN_EXTERN void pn_url_clear(pn_url_t *url) {
pn_string_clear(url->str);
}
+/** URL-encode src and append to dst. */
+static void pni_urlencode(pn_string_t *dst, const char* src) {
+ static const char *bad = "@:/";
+
+ if (!src) return;
+ const char *i = src;
+ const char *j = strpbrk(i, bad);
+ while (j) {
+ pn_string_addf(dst, "%.*s", (int)(j-i), i);
+ pn_string_addf(dst, "%%%02X", (int)*j);
+ i = j + 1;
+ j = strpbrk(i, bad);
+ }
+ pn_string_addf(dst, "%s", i);
+}
+
+
/** Return the string form of a URL. */
PN_EXTERN const char *pn_url_str(pn_url_t *url) {
if (pn_string_get(url->str) == NULL) {
pn_string_set(url->str, "");
if (url->scheme) pn_string_addf(url->str, "%s://", url->scheme);
- if (url->username) pn_string_addf(url->str, "%s", url->username);
- if (url->password) pn_string_addf(url->str, ":%s", url->password);
+ if (url->username) pni_urlencode(url->str, url->username);
+ if (url->password) {
+ pn_string_addf(url->str, ":");
+ pni_urlencode(url->str, url->password);
+ }
if (url->username || url->password) pn_string_addf(url->str, "@");
if (url->host) {
if (strchr(url->host, ':')) pn_string_addf(url->str, "[%s]", url->host);
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/aa226be3/proton-c/src/util.c
----------------------------------------------------------------------
diff --git a/proton-c/src/util.c b/proton-c/src/util.c
index e2c6727..0587c1d 100644
--- a/proton-c/src/util.c
+++ b/proton-c/src/util.c
@@ -135,13 +135,6 @@ void pni_urldecode(const char *src, char *dst)
*out = '\0';
}
-// Parse URL syntax:
-// [ <scheme> :// ] [ <user> [ : <password> ] @ ] <host> [ : <port> ] [ / <path> ]
-// <scheme>, <user>, <password>, <port> cannot contain any of '@', ':', '/'
-// If the first character of <host> is '[' then it can contain any character up to ']' (this is to allow IPv6
-// literal syntax). Otherwise it also cannot contain '@', ':', '/'
-// <host> is not optional but it can be null! If it is not present an empty string will be returned
-// <path> can contain any character
void pni_parse_url(char *url, char **scheme, char **user, char **pass, char **host, char **port, char **path)
{
if (!url) return;
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/aa226be3/tests/python/proton_tests/interop.py
----------------------------------------------------------------------
diff --git a/tests/python/proton_tests/interop.py b/tests/python/proton_tests/interop.py
index bb86535..b56298b 100644
--- a/tests/python/proton_tests/interop.py
+++ b/tests/python/proton_tests/interop.py
@@ -25,7 +25,7 @@ from proton._compat import str2bin
def find_test_interop_dir():
"""Walk up the directory tree to find the tests directory."""
- f = os.path.dirname(__file__)
+ f = os.path.dirname(os.path.abspath(__file__))
while f and os.path.basename(f) != "tests": f = os.path.dirname(f)
f = os.path.join(f, "interop")
if not os.path.isdir(f):
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org