From 66a4ff496afe63305cb1a437bcb52d1efc11edf7 Mon Sep 17 00:00:00 2001 From: David Steele Date: Fri, 19 Feb 2021 09:05:32 -0500 Subject: [PATCH] Encode path before passing to HttpRequest. GCS requires mixed encoding in the path so encoding inside HttpRequest does not work. Instead, require the path to be correctly encoded before being passed to HttpRequest. --- src/common/io/http/request.c | 4 ++-- src/storage/azure/storage.c | 7 +++++-- src/storage/s3/storage.c | 5 ++++- test/src/module/common/ioHttpTest.c | 10 +++++++--- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/common/io/http/request.c b/src/common/io/http/request.c index 70150a142..5407dc6f4 100644 --- a/src/common/io/http/request.c +++ b/src/common/io/http/request.c @@ -108,7 +108,7 @@ httpRequestProcess(HttpRequest *this, bool waitForResponse, bool contentCache) String *requestStr = strNewFmt( "%s %s%s%s " HTTP_VERSION CRLF_Z HTTP_HEADER_USER_AGENT ":" PROJECT_NAME "/" PROJECT_VERSION CRLF_Z, - strZ(this->verb), strZ(httpUriEncode(this->path, true)), this->query == NULL ? "" : "?", + strZ(this->verb), strZ(this->path), this->query == NULL ? "" : "?", this->query == NULL ? "" : strZ(httpQueryRenderP(this->query))); // Add headers @@ -257,7 +257,7 @@ httpRequestError(const HttpRequest *this, HttpResponse *response) // Output path/query strCatZ(error, ":\n*** Path/Query ***:"); - strCatFmt(error, "\n%s", strZ(httpUriEncode(this->path, true))); + strCatFmt(error, "\n%s", strZ(this->path)); if (this->query != NULL) strCatFmt(error, "?%s", strZ(httpQueryRenderP(this->query, .redact = true))); diff --git a/src/storage/azure/storage.c b/src/storage/azure/storage.c index dabbe11de..36a50f88f 100644 --- a/src/storage/azure/storage.c +++ b/src/storage/azure/storage.c @@ -234,6 +234,9 @@ storageAzureRequestAsync(StorageAzure *this, const String *verb, StorageAzureReq httpHeaderAdd(requestHeader, HTTP_HEADER_CONTENT_MD5_STR, STR(md5Hash)); } + // Encode path + const String *const path = httpUriEncode(param.path, true); + // Make a copy of the query so it can be modified HttpQuery *query = this->sasKey != NULL && param.query == NULL ? @@ -241,13 +244,13 @@ storageAzureRequestAsync(StorageAzure *this, const String *verb, StorageAzureReq httpQueryDupP(param.query, .redactList = this->queryRedactList); // Generate authorization header - storageAzureAuth(this, verb, httpUriEncode(param.path, true), query, httpDateFromTime(time(NULL)), requestHeader); + storageAzureAuth(this, verb, path, query, httpDateFromTime(time(NULL)), requestHeader); // Send request MEM_CONTEXT_PRIOR_BEGIN() { result = httpRequestNewP( - this->httpClient, verb, param.path, .query = query, .header = requestHeader, .content = param.content); + this->httpClient, verb, path, .query = query, .header = requestHeader, .content = param.content); } MEM_CONTEXT_END(); } diff --git a/src/storage/s3/storage.c b/src/storage/s3/storage.c index ccab3b497..982541572 100644 --- a/src/storage/s3/storage.c +++ b/src/storage/s3/storage.c @@ -407,9 +407,12 @@ storageS3RequestAsync(StorageS3 *this, const String *verb, const String *path, S this->signingKeyDate = YYYYMMDD_STR; } + // Encode path + path = httpUriEncode(path, true); + // Generate authorization header storageS3Auth( - this, verb, httpUriEncode(path, true), param.query, storageS3DateTime(time(NULL)), requestHeader, + this, verb, path, param.query, storageS3DateTime(time(NULL)), requestHeader, param.content == NULL || bufEmpty(param.content) ? HASH_TYPE_SHA256_ZERO_STR : bufHex(cryptoHashOne(HASH_TYPE_SHA256_STR, param.content))); diff --git a/test/src/module/common/ioHttpTest.c b/test/src/module/common/ioHttpTest.c index ec0ac754e..3d8080606 100644 --- a/test/src/module/common/ioHttpTest.c +++ b/test/src/module/common/ioHttpTest.c @@ -546,7 +546,7 @@ testRun(void) response, httpRequestResponse( httpRequestNewP( - client, strNew("GET"), strNew("/path/file 1.txt"), + client, strNew("GET"), httpUriEncode(strNew("/path/file 1.txt"), true), .header = httpHeaderAdd(httpHeaderNew(NULL), strNew("content-length"), strNew("30")), .content = BUFSTRDEF("012345678901234567890123456789")), true), "request"); @@ -570,7 +570,9 @@ testRun(void) hrnServerScriptReplyZ(http, "HTTP/1.1 200 OK\r\ncontent-length:32\r\n\r\n01234567890123456789012345678901"); TEST_ASSIGN( - response, httpRequestResponse(httpRequestNewP(client, strNew("GET"), strNew("/path/file 1.txt")), true), + response, + httpRequestResponse( + httpRequestNewP(client, strNew("GET"), httpUriEncode(strNew("/path/file 1.txt"), true)), true), "request"); TEST_RESULT_STR_Z(strNewBuf(httpResponseContent(response)), "01234567890123456789012345678901", "check response"); TEST_RESULT_UINT(httpResponseRead(response, bufNew(1), true), 0, "call internal read to check eof"); @@ -584,7 +586,9 @@ testRun(void) hrnServerScriptClose(http); TEST_ASSIGN( - response, httpRequestResponse(httpRequestNewP(client, strNew("GET"), strNew("/path/file 1.txt")), false), + response, + httpRequestResponse( + httpRequestNewP(client, strNew("GET"), httpUriEncode(strNew("/path/file 1.txt"), true)), false), "request"); TEST_RESULT_PTR_NE(response->session, NULL, "session is busy"); TEST_ERROR(ioRead(httpResponseIoRead(response), bufNew(32)), FileReadError, "unexpected EOF reading HTTP content");