From 022963c08bf04c42aa942dc7a538dedacaf0d071 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Wed, 12 Jun 2024 07:18:25 -0700 Subject: [PATCH 1/6] [FIXED] Support %-encoding in username, password when parsing connect URLs --- src/url.c | 41 +++++++++++++++++++++++++++++++++++++++-- test/test.c | 19 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/url.c b/src/url.c index 4e5910d0..ddcce91c 100644 --- a/src/url.c +++ b/src/url.c @@ -15,6 +15,7 @@ #include "util.h" #include +#include #include "mem.h" @@ -46,6 +47,42 @@ _parsePort(int *port, const char *sport) return s; } +static natsStatus _decodeAndDup(char **decoded, const char *encoded) +{ + size_t len = strlen(encoded); + const char *p = encoded; + const char *e = encoded + len; + char *d; + + *decoded = NATS_MALLOC(len + 1); + if (*decoded == NULL) + { + return nats_setDefaultError(NATS_NO_MEMORY); + } + d = *decoded; + for (; p < e; p++) + { + if (*p != '%') + { + *d++ = *p; + continue; + } + + if (e - p < 3 || (!isxdigit(*(p + 1)) || !isxdigit(*(p + 2)))) + { + NATS_FREE(*decoded); + *decoded = NULL; + return nats_setError(NATS_ERR, "invalid percent encoding in URL: %s", encoded); + } + + char buf[3] = {p[1], p[2], '\0'}; + *d++ = (char)strtol(buf, NULL, 16); + p += 2; + } + *d = '\0'; + return NATS_OK; +} + natsStatus natsUrl_Create(natsUrl **newUrl, const char *urlStr) { @@ -166,9 +203,9 @@ natsUrl_Create(natsUrl **newUrl, const char *urlStr) DUP_STRING(s, url->host, host); if (user != NULL) - IF_OK_DUP_STRING(s, url->username, user); + IFOK(s, _decodeAndDup(&url->username, user)); if (pwd != NULL) - IF_OK_DUP_STRING(s, url->password, pwd); + IFOK(s, _decodeAndDup(&url->password, pwd)); if ((s == NATS_OK) && nats_asprintf(&url->fullUrl, "%s://%s%s%s%s%s:%d%s%s", scheme, userval, usep, pwdval, hsep, host, url->port, pathsep, pathval) < 0) diff --git a/test/test.c b/test/test.c index ec2d2e88..346b5b7d 100644 --- a/test/test.c +++ b/test/test.c @@ -1764,6 +1764,25 @@ test_natsUrl(void) natsUrl_Destroy(u); u = NULL; + test("'tcp://%4C%65v:p%77d@localhost':"); + s = natsUrl_Create(&u, "tcp://%4C%65v:p%77d@localhost"); + testCond((s == NATS_OK) + && (u != NULL) + && (u->host != NULL) + && (strcmp(u->host, "localhost") == 0) + && (u->username != NULL) + && (strcmp(u->username, "Lev") == 0) + && (u->password != NULL) + && (strcmp(u->password, "pwd") == 0) + && (u->port == 4222)); + natsUrl_Destroy(u); + u = NULL; + + test("'tcp://%4c%65v:p%@localhost':"); + s = natsUrl_Create(&u, "tcp://%4%65v:p%@localhost"); + testCond((s == NATS_ERR) && (u == NULL) && (strstr(nats_GetLastError(NULL), "invalid percent encoding in URL: %4%65v") != NULL)); + nats_clearLastError(); + test("'tcp://localhost: 4222':"); s = natsUrl_Create(&u, "tcp://localhost: 4222"); testCond((s == NATS_INVALID_ARG) From aa8def0c3a1d55f48a6be212234020a095c6b999 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Wed, 12 Jun 2024 07:36:19 -0700 Subject: [PATCH 2/6] better test --- test/test.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/test.c b/test/test.c index 346b5b7d..859e3f51 100644 --- a/test/test.c +++ b/test/test.c @@ -1779,8 +1779,13 @@ test_natsUrl(void) u = NULL; test("'tcp://%4c%65v:p%@localhost':"); - s = natsUrl_Create(&u, "tcp://%4%65v:p%@localhost"); - testCond((s == NATS_ERR) && (u == NULL) && (strstr(nats_GetLastError(NULL), "invalid percent encoding in URL: %4%65v") != NULL)); + s = natsUrl_Create(&u, "tcp://%4c%65v:p%@localhost"); + testCond((s == NATS_ERR) && (u == NULL) && (strstr(nats_GetLastError(NULL), "invalid percent encoding in URL: p%") != NULL)); + nats_clearLastError(); + + test("'tcp://%4H%65v:p%@localhost':"); + s = natsUrl_Create(&u, "tcp://%4H%65v:p%@localhost"); + testCond((s == NATS_ERR) && (u == NULL) && (strstr(nats_GetLastError(NULL), "invalid percent encoding in URL: %4H%65v") != NULL)); nats_clearLastError(); test("'tcp://localhost: 4222':"); From 8bdde702d69826fc7dfe528dc33151cdca8b05e0 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Wed, 12 Jun 2024 23:16:14 -0700 Subject: [PATCH 3/6] [TEST ONLY] Fixed Test_JetStreamSubscribeIdleHeartbeat --- test/test.c | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/test/test.c b/test/test.c index ec2d2e88..1b1566ee 100644 --- a/test/test.c +++ b/test/test.c @@ -27912,7 +27912,7 @@ test_JetStreamSubscribeIdleHearbeat(void) natsMutex_Unlock(args.m); testCond(s == NATS_OK); - test("Check HB received: "); + test("Wait and check HB received: "); nats_Sleep(300); natsSubAndLdw_Lock(sub); s = (sub->jsi->mismatch.dseq == 1 ? NATS_OK : NATS_ERR); @@ -27951,18 +27951,35 @@ test_JetStreamSubscribeIdleHearbeat(void) s = js_Publish(NULL, js, "foo", "msg2", 4, NULL, &jerr); testCond((s == NATS_OK) && (jerr == 0)); + // Cheat by pretending that the server sends message seq 3, while client + // received only seq 1. Disable auto-ack for this message, or we mess up the + // server state. +#define PUBLISH_FAKE_JS_MSG_WITH_SEQ(_reply, _msg) \ + { \ + natsSub_Lock(sub); \ + inbox = sub->subject; \ + sub->jsi->ackNone = true; \ + natsSub_Unlock(sub); \ + \ + natsConn_setFilterWithClosure(nc, _setMsgReply, (void *)(_reply)); \ + s = natsConnection_PublishString(nc, inbox, (_msg)); \ + } + +#define PUBLISH_FAKE_RESET() \ + { \ + natsSub_Lock(sub); \ + sub->jsi->ackNone = true; \ + natsSub_Unlock(sub); \ + } + test("Check seq mismatch: "); - natsSub_Lock(sub); - inbox = sub->subject; - natsSub_Unlock(sub); - // Cheat by pretending that the server sends message seq 3, while client received only seq 1. - natsConn_setFilterWithClosure(nc, _setMsgReply, (void*) "$JS.ACK.TEST.dur1.1.3.3.1624472520000000000.0"); - s = natsConnection_PublishString(nc, inbox, "msg3"); + PUBLISH_FAKE_JS_MSG_WITH_SEQ("$JS.ACK.TEST.dur1.1.3.3.1624472520000000000.0", "msg3 fake"); // Wait for past the next HB and we should get an async error natsMutex_Lock(args.m); while ((s != NATS_TIMEOUT) && !args.done) s = natsCondition_TimedWait(args.c, args.m, 300); natsMutex_Unlock(args.m); + PUBLISH_FAKE_RESET(); testCond(s == NATS_OK); test("Check that notification is sent only once: "); @@ -27996,8 +28013,7 @@ test_JetStreamSubscribeIdleHearbeat(void) testCond(s == NATS_OK); test("Skip again: "); - natsConn_setFilterWithClosure(nc, _setMsgReply, (void*) "$JS.ACK.TEST.dur1.1.4.4.1624482520000000000.0"); - s = natsConnection_PublishString(nc, inbox, "msg4"); + PUBLISH_FAKE_JS_MSG_WITH_SEQ("$JS.ACK.TEST.dur1.1.4.4.1624482520000000000.0", "msg4 fake"); testCond(s == NATS_OK); test("Check async cb invoked: "); @@ -28005,6 +28021,7 @@ test_JetStreamSubscribeIdleHearbeat(void) while ((s != NATS_TIMEOUT) && !args.done) s = natsCondition_TimedWait(args.c, args.m, 1000); natsMutex_Unlock(args.m); + PUBLISH_FAKE_RESET(); testCond(s == NATS_OK); test("Check HB timer reports missed HB: "); @@ -28059,11 +28076,7 @@ test_JetStreamSubscribeIdleHearbeat(void) nats_clearLastError(); test("Skip: "); - natsSub_Lock(sub); - inbox = sub->subject; - natsSub_Unlock(sub); - natsConn_setFilterWithClosure(nc, _setMsgReply, (void*) "$JS.ACK.TEST.dur2.1.4.4.1624482520000000000.0"); - s = natsConnection_PublishString(nc, inbox, "msg4"); + PUBLISH_FAKE_JS_MSG_WITH_SEQ("$JS.ACK.TEST.dur2.1.4.4.1624482520000000000.0", "msg4 fake"); testCond(s == NATS_OK); // For sync subs, we should not get async error @@ -28074,6 +28087,7 @@ test_JetStreamSubscribeIdleHearbeat(void) natsMutex_Unlock(args.m); testCond(s == NATS_TIMEOUT); nats_clearLastError(); + PUBLISH_FAKE_RESET(); test("NextMsg reports error: "); s = natsSubscription_NextMsg(&msg, sub, 1000); @@ -28103,8 +28117,7 @@ test_JetStreamSubscribeIdleHearbeat(void) testCond(s == NATS_OK); test("Skip again: "); - natsConn_setFilterWithClosure(nc, _setMsgReply, (void*) "$JS.ACK.TEST.dur1.1.5.5.1624492520000000000.0"); - s = natsConnection_PublishString(nc, inbox, "msg5"); + PUBLISH_FAKE_JS_MSG_WITH_SEQ("$JS.ACK.TEST.dur1.1.5.5.1624492520000000000.0", "msg5 fake"); testCond(s == NATS_OK); test("NextMsg reports error: "); @@ -28112,6 +28125,7 @@ test_JetStreamSubscribeIdleHearbeat(void) s = natsSubscription_NextMsg(&msg, sub, 1000); testCond((s == NATS_MISMATCH) && (msg == NULL)); nats_clearLastError(); + PUBLISH_FAKE_RESET(); test("Check HB timer reports missed HB: "); s = NATS_OK; From 20b1fee9a5834bf77290bc047b0eca1b71de1817 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Wed, 12 Jun 2024 23:28:35 -0700 Subject: [PATCH 4/6] Nit: removed an irrelevant format change --- test/test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.c b/test/test.c index 1b1566ee..24b2c03d 100644 --- a/test/test.c +++ b/test/test.c @@ -27912,7 +27912,7 @@ test_JetStreamSubscribeIdleHearbeat(void) natsMutex_Unlock(args.m); testCond(s == NATS_OK); - test("Wait and check HB received: "); + test("Check HB received: "); nats_Sleep(300); natsSubAndLdw_Lock(sub); s = (sub->jsi->mismatch.dseq == 1 ? NATS_OK : NATS_ERR); From 92b9c7d973408d929c90a7ee7b40348f62dbf2d7 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Thu, 13 Jun 2024 07:27:08 -0700 Subject: [PATCH 5/6] PR feedback: typo --- test/test.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test.c b/test/test.c index 24b2c03d..71f2a5b9 100644 --- a/test/test.c +++ b/test/test.c @@ -27965,11 +27965,11 @@ test_JetStreamSubscribeIdleHearbeat(void) s = natsConnection_PublishString(nc, inbox, (_msg)); \ } -#define PUBLISH_FAKE_RESET() \ - { \ - natsSub_Lock(sub); \ - sub->jsi->ackNone = true; \ - natsSub_Unlock(sub); \ +#define PUBLISH_FAKE_RESET() \ + { \ + natsSub_Lock(sub); \ + sub->jsi->ackNone = false; \ + natsSub_Unlock(sub); \ } test("Check seq mismatch: "); From b3d247e1b9c13e2343b8958dc629a2c4080f7815 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Thu, 13 Jun 2024 08:22:44 -0700 Subject: [PATCH 6/6] PR feedback: include %00 in the test --- test/test.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/test.c b/test/test.c index 75f36fec..048f72d7 100644 --- a/test/test.c +++ b/test/test.c @@ -1764,8 +1764,8 @@ test_natsUrl(void) natsUrl_Destroy(u); u = NULL; - test("'tcp://%4C%65v:p%77d@localhost':"); - s = natsUrl_Create(&u, "tcp://%4C%65v:p%77d@localhost"); + test("'tcp://%4C%65v%00ignored:p%77d%00ignoredalso@localhost':"); + s = natsUrl_Create(&u, "tcp://%4C%65v%00ignored:p%77d%00ignoredalso@localhost"); testCond((s == NATS_OK) && (u != NULL) && (u->host != NULL) @@ -1778,6 +1778,12 @@ test_natsUrl(void) natsUrl_Destroy(u); u = NULL; + test("'tcp://%4C%65v:p%77d@localhost':"); + s = natsUrl_Create(&u, "tcp://%4C%65v:p%77d@localhost"); + testCond((s == NATS_OK) && (u != NULL) && (u->host != NULL) && (strcmp(u->host, "localhost") == 0) && (u->username != NULL) && (strcmp(u->username, "Lev") == 0) && (u->password != NULL) && (strcmp(u->password, "pwd") == 0) && (u->port == 4222)); + natsUrl_Destroy(u); + u = NULL; + test("'tcp://%4c%65v:p%@localhost':"); s = natsUrl_Create(&u, "tcp://%4c%65v:p%@localhost"); testCond((s == NATS_ERR) && (u == NULL) && (strstr(nats_GetLastError(NULL), "invalid percent encoding in URL: p%") != NULL));