db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-03-03 14:52:21 +02:00
Code Issues Releases Activity

Improve handling of invalid HTTP response status.

Browse Source 
A truncated HTTP response status could lead to an an unfriendly error message, which would be retried, but could be confusing if the error was persistent and required debugging.

Improve the error handling overall to catch more error cases explicitly and respond better to edge cases.

Also update the terminology in comments to align with the RFC. Variable and function names were not changed because a refactor is intended for HTTP response and it doesn't seem worth the additional code churn.
This commit is contained in:
David Steele 2020-05-27 15:13:55 -04:00 committed by GitHub
parent d05090ab7b
commit 3b5f76b434
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
doc/xml/release.xml
View File

@ -13,6 +13,17 @@
<release-list>
<release date="XXXX-XX-XX" version="2.28dev" title="UNDER DEVELOPMENT">
<release-core-list>
<release-improvement-list>
<release-item>
<release-item-contributor-list>
<release-item-reviewer id="cynthia.shang"/>
</release-item-contributor-list>
<p>Improve handling of invalid HTTP response status.</p>
</release-item>
</release-improvement-list>
</release-core-list>
</release>
<release date="2020-05-26" version="2.27" title="Expiration Improvements and Compression Drivers">

28
src/common/io/http/client.c
View File

@ -319,22 +319,34 @@ httpClientRequest(
// Flush all writes
ioWriteFlush(tlsSessionIoWrite(this->tlsSession));
// Read status and make sure it starts with the correct http version
// Read status
String *status = ioReadLine(tlsSessionIoRead(this->tlsSession));
if (!strBeginsWith(status, HTTP_VERSION_STR))
THROW_FMT(FormatError, "http version of response '%s' must be " HTTP_VERSION, strPtr(strTrim(status)));
// Check status ends with a CR and remove it to make error formatting easier and more accurate
if (!strEndsWith(status, CR_STR))
THROW_FMT(FormatError, "http response status '%s' should be CR-terminated", strPtr(status));
// Now read the response code and message (and strip the trailing CR)
status = strSubN(status, sizeof(HTTP_VERSION), strSize(status) - sizeof(HTTP_VERSION) - 1);
status = strSubN(status, 0, strSize(status) - 1);
// Check status is at least the minimum required length to avoid harder to interpret errors later on
if (strSize(status) < sizeof(HTTP_VERSION) + 4)
THROW_FMT(FormatError, "http response '%s' has invalid length", strPtr(strTrim(status)));
// Check status starts with the correct http version
if (!strBeginsWith(status, HTTP_VERSION_STR))
THROW_FMT(FormatError, "http version of response '%s' must be " HTTP_VERSION, strPtr(status));
// Read status code
status = strSub(status, sizeof(HTTP_VERSION));
int spacePos = strChr(status, ' ');
if (spacePos < 0)
THROW_FMT(FormatError, "response status '%s' must have a space", strPtr(status));
if (spacePos != 3)
THROW_FMT(FormatError, "response status '%s' must have a space after the status code", strPtr(status));
this->responseCode = cvtZToUInt(strPtr(strTrim(strSubN(status, 0, (size_t)spacePos))));
this->responseCode = cvtZToUInt(strPtr(strSubN(status, 0, (size_t)spacePos)));
// Read reason phrase. A missing reason phrase will be represented as an empty string.
MEM_CONTEXT_BEGIN(this->memContext)
{
this->responseMessage = strSub(status, (size_t)spacePos + 1);

30
test/src/module/common/ioHttpTest.c
View File

@ -182,6 +182,34 @@ testRun(void)
httpClientRequest(client, strNew("GET"), strNew("/"), NULL, NULL, NULL, false), FileReadError,
"unexpected eof while reading line");
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("no CR at end of status");
hrnTlsServerAccept();
hrnTlsServerExpectZ("GET / HTTP/1.1\r\n\r\n");
hrnTlsServerReplyZ("HTTP/1.0 200 OK\n");
hrnTlsServerClose();
TEST_ERROR(
httpClientRequest(client, strNew("GET"), strNew("/"), NULL, NULL, NULL, false), FormatError,
"http response status 'HTTP/1.0 200 OK' should be CR-terminated");
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("status too short");
hrnTlsServerAccept();
hrnTlsServerExpectZ("GET / HTTP/1.1\r\n\r\n");
hrnTlsServerReplyZ("HTTP/1.0 200\r\n");
hrnTlsServerClose();
TEST_ERROR(
httpClientRequest(client, strNew("GET"), strNew("/"), NULL, NULL, NULL, false), FormatError,
"http response 'HTTP/1.0 200' has invalid length");
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("invalid http version");
@ -208,7 +236,7 @@ testRun(void)
TEST_ERROR(
httpClientRequest(client, strNew("GET"), strNew("/"), NULL, NULL, NULL, false), FormatError,
"response status '200OK' must have a space");
"response status '200OK' must have a space after the status code");
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("unexpected end of headers");