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 <[email protected]> Authored: Thu Nov 19 12:36:53 2015 -0500 Committer: Alan Conway <[email protected]> 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%[email protected]/some_topic", + "amqps", "@user/:", "@pass/:", "example.net", 0, "some_topic")); + TEST(testrt("amqps://user%2F%3A=:pass%2F%[email protected]/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: [email protected] For additional commands, e-mail: [email protected]
