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

Fix C code to recognize host:port format like Perl does.

Browse Source 
This was not an intentional feature in Perl, but it works, so it makes sense to implement the same syntax in C.

This is a break from other places where a -port option is explicitly supplied, so it may make sense to support both styles going forward.  This commit does not address that, however.

Reported by Kyle Nevins.
This commit is contained in:
David Steele 2019-04-10 17:48:34 -04:00
parent 3aa521fed0
commit df12cbb162
8 changed files with 196 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
doc/xml/release.xml
View File

@ -33,6 +33,14 @@
<p>Fix issue setting <br-option>log-level-file=off</br-option> for the <cmd>archive-get</cmd> command.</p>
</release-item>
<release-item>
<release-item-contributor-list>
<release-item-ideator id="kyle.nevins"/>
</release-item-contributor-list>
<p>Fix C code to recognize <id>host:port</id> option format like Perl does.</p>
</release-item>
<release-item>
<p>Fix issues with <code>remote</code>/<code>local</code> command logging options.</p>
</release-item>
@ -6804,6 +6812,11 @@
<contributor-id type="github">keithf4</contributor-id>
</contributor>
<contributor id="kyle.nevins">
<contributor-name-display>Kyle Nevins</contributor-name-display>
<contributor-id type="github">kyle-nevins</contributor-id>
</contributor>
<contributor id="laetitia">
<contributor-name-display>L&amp;aelig;titia</contributor-name-display>
<contributor-id type="github">LaetitiaLoxo</contributor-id>

72
src/config/config.c
View File

@ -545,6 +545,78 @@ cfgOptionDefaultSet(ConfigOption optionId, const Variant *defaultValue)
FUNCTION_TEST_RETURN_VOID();
}
/***********************************************************************************************************************************
Parse a host option and extract the host and port (if it exists)
***********************************************************************************************************************************/
String *
cfgOptionHostPort(ConfigOption optionId, unsigned int *port)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(ENUM, optionId);
FUNCTION_TEST_PARAM_P(UINT, port);
FUNCTION_TEST_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(port != NULL);
String *result = NULL;
// Proceed if option is valid and has a value
if (cfgOptionTest(optionId))
{
MEM_CONTEXT_TEMP_BEGIN()
{
const String *host = cfgOptionStr(optionId);
// If the host contains a colon then it has a port appended
if (strChr(host, ':') != -1)
{
const StringList *hostPart = strLstNewSplitZ(host, ":");
// More than one colon is invalid
if (strLstSize(hostPart) > 2)
{
THROW_FMT(
OptionInvalidError,
"'%s' is not valid for option '%s'"
"\nHINT: is more than one port specified?",
strPtr(host), cfgOptionName(optionId));
}
// Set the host
memContextSwitch(MEM_CONTEXT_OLD());
result = strDup(strLstGet(hostPart, 0));
memContextSwitch(MEM_CONTEXT_TEMP());
// Set the port and error if it is not a positive integer
TRY_BEGIN()
{
*port = cvtZToUInt(strPtr(strLstGet(hostPart, 1)));
}
CATCH(FormatError)
{
THROW_FMT(
OptionInvalidError,
"'%s' is not valid for option '%s'"
"\nHINT: port is not a positive integer.",
strPtr(host), cfgOptionName(optionId));
}
TRY_END();
}
// Else there is no port and just copy the host
else
{
memContextSwitch(MEM_CONTEXT_OLD());
result = strDup(host);
memContextSwitch(MEM_CONTEXT_TEMP());
}
}
MEM_CONTEXT_TEMP_END();
}
FUNCTION_TEST_RETURN(result);
}
/***********************************************************************************************************************************
Get index for option
***********************************************************************************************************************************/

1
src/config/config.h
View File

@ -100,6 +100,7 @@ void cfgExeSet(const String *exeParam);
const Variant *cfgOptionDefault(ConfigOption optionId);
void cfgOptionDefaultSet(ConfigOption optionId, const Variant *defaultValue);
ConfigDefineOption cfgOptionDefIdFromId(ConfigOption optionId);
String *cfgOptionHostPort(ConfigOption optionId, unsigned int *port);
int cfgOptionId(const char *optionName);
ConfigOption cfgOptionIdFromDefId(ConfigDefineOption optionDefId, unsigned int index);
bool cfgOptionNegate(ConfigOption optionId);

4
src/storage/driver/s3/storage.c
View File

@ -78,6 +78,7 @@ struct StorageDriverS3
const String *securityToken; // Security token, if any
size_t partSize; // Part size for multi-part upload
const String *host; // Defaults to {bucket}.{endpoint}
unsigned int port; // Host port
// Current signing key and date it is valid for
const String *signingKeyDate; // Date of cached signing key (so we know when to regenerate)
@ -261,6 +262,7 @@ storageDriverS3New(
this->securityToken = strDup(securityToken);
this->partSize = partSize;
this->host = host == NULL ? strNewFmt("%s.%s", strPtr(bucket), strPtr(endPoint)) : strDup(host);
this->port = port;
// Force the signing key to be generated on the first run
this->signingKeyDate = YYYYMMDD_STR;
@ -276,7 +278,7 @@ storageDriverS3New(
.pathSync = (StorageInterfacePathSync)storageDriverS3PathSync, .remove = (StorageInterfaceRemove)storageDriverS3Remove);
// Create the http client used to service requests
this->httpClient = httpClientNew(this->host, port, timeout, verifyPeer, caFile, caPath);
this->httpClient = httpClientNew(this->host, this->port, timeout, verifyPeer, caFile, caPath);
this->headerRedactList = strLstAdd(strLstNew(), S3_HEADER_AUTHORIZATION_STR);
}
MEM_CONTEXT_NEW_END();

14
src/storage/helper.c
View File

@ -302,13 +302,19 @@ storageRepoGet(const String *type, bool write)
// Use the S3 driver
else if (strEqZ(type, STORAGE_TYPE_S3))
{
// Set the default port
unsigned int port = STORAGE_DRIVER_S3_PORT_DEFAULT;
// Extract port from the endpoint and host if it is present
const String *endPoint = cfgOptionHostPort(cfgOptRepoS3Endpoint, &port);
const String *host = cfgOptionHostPort(cfgOptRepoS3Host, &port);
result = storageDriverS3Interface(
storageDriverS3New(
cfgOptionStr(cfgOptRepoPath), write, storageRepoPathExpression, cfgOptionStr(cfgOptRepoS3Bucket),
cfgOptionStr(cfgOptRepoS3Endpoint), cfgOptionStr(cfgOptRepoS3Region), cfgOptionStr(cfgOptRepoS3Key),
cfgOptionStr(cfgOptRepoS3KeySecret), cfgOptionTest(cfgOptRepoS3Token) ? cfgOptionStr(cfgOptRepoS3Token) : NULL,
(size_t)5 * 1024 * 1024, cfgOptionTest(cfgOptRepoS3Host) ? cfgOptionStr(cfgOptRepoS3Host) : NULL,
STORAGE_DRIVER_S3_PORT_DEFAULT, STORAGE_DRIVER_S3_TIMEOUT_DEFAULT, cfgOptionBool(cfgOptRepoS3VerifySsl),
endPoint, cfgOptionStr(cfgOptRepoS3Region), cfgOptionStr(cfgOptRepoS3Key), cfgOptionStr(cfgOptRepoS3KeySecret),
cfgOptionTest(cfgOptRepoS3Token) ? cfgOptionStr(cfgOptRepoS3Token) : NULL, (size_t)5 * 1024 * 1024, host, port,
STORAGE_DRIVER_S3_TIMEOUT_DEFAULT, cfgOptionBool(cfgOptRepoS3VerifySsl),
cfgOptionTest(cfgOptRepoS3CaFile) ? cfgOptionStr(cfgOptRepoS3CaFile) : NULL,
cfgOptionTest(cfgOptRepoS3CaPath) ? cfgOptionStr(cfgOptRepoS3CaPath) : NULL));
}

2
test/define.yaml
View File

@ -375,7 +375,7 @@ unit:
# ----------------------------------------------------------------------------------------------------------------------------
- name: config
total: 3
total: 4
coverage:
config/config: full

36
test/src/module/config/configTest.c
View File

@ -194,6 +194,42 @@ testRun(void)
TEST_RESULT_INT(cfgCommand(), cfgCmdNone, "command begins as none");
}
// *****************************************************************************************************************************
if (testBegin("cfgOptionHostPort()"))
{
unsigned int port = 55555;
cfgInit();
cfgCommandSet(cfgCmdBackup);
cfgOptionValidSet(cfgOptRepoS3Host, true);
cfgOptionSet(cfgOptRepoS3Host, cfgSourceConfig, varNewStrZ("host.com")) ;
TEST_RESULT_STR(strPtr(cfgOptionHostPort(cfgOptRepoS3Host, &port)), "host.com", "check plain host");
TEST_RESULT_UINT(port, 55555, "check that port was not updated");
cfgOptionSet(cfgOptRepoS3Host, cfgSourceConfig, varNewStrZ("myhost.com:777")) ;
TEST_RESULT_STR(strPtr(cfgOptionHostPort(cfgOptRepoS3Host, &port)), "myhost.com", "check host with port");
TEST_RESULT_UINT(port, 777, "check that port was updated");
TEST_RESULT_STR(strPtr(cfgOptionHostPort(cfgOptRepoS3Endpoint, &port)), NULL, "check null host");
TEST_RESULT_UINT(port, 777, "check that port was not updated");
cfgOptionSet(cfgOptRepoS3Host, cfgSourceConfig, varNewStrZ("myhost.com:777:888")) ;
TEST_ERROR(
cfgOptionHostPort(cfgOptRepoS3Host, &port), OptionInvalidError,
"'myhost.com:777:888' is not valid for option 'repo1-s3-host'"
"\nHINT: is more than one port specified?");
TEST_RESULT_UINT(port, 777, "check that port was not updated");
cfgOptionValidSet(cfgOptRepoS3Endpoint, true);
cfgOptionSet(cfgOptRepoS3Endpoint, cfgSourceConfig, varNewStrZ("myendpoint.com:ZZZ")) ;
TEST_ERROR(
cfgOptionHostPort(cfgOptRepoS3Endpoint, &port), OptionInvalidError,
"'myendpoint.com:ZZZ' is not valid for option 'repo1-s3-endpoint'"
"\nHINT: port is not a positive integer.");
TEST_RESULT_UINT(port, 777, "check that port was not updated");
}
// *****************************************************************************************************************************
if (testBegin("cfgOptionDefault() and cfgOptionDefaultSet()"))
{

60
test/src/module/storage/s3Test.c
View File

@ -384,6 +384,66 @@ testRun(void)
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->bucket), strPtr(bucket), " check bucket");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->region), strPtr(region), " check region");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->host), strPtr(host), " check host");
TEST_RESULT_UINT(((StorageDriverS3 *)storage->driver)->port, 443, " check port");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->accessKey), strPtr(accessKey), " check access key");
TEST_RESULT_STR(
strPtr(((StorageDriverS3 *)storage->driver)->secretAccessKey), strPtr(secretAccessKey), " check secret access key");
TEST_RESULT_STR(
strPtr(((StorageDriverS3 *)storage->driver)->securityToken), strPtr(securityToken), " check security token");
// Add a port to the endpoint
// -------------------------------------------------------------------------------------------------------------------------
argList = strLstNew();
strLstAddZ(argList, "pgbackrest");
strLstAddZ(argList, "--stanza=db");
strLstAddZ(argList, "--repo1-type=s3");
strLstAdd(argList, strNewFmt("--repo1-path=%s", strPtr(path)));
strLstAdd(argList, strNewFmt("--repo1-s3-bucket=%s", strPtr(bucket)));
strLstAdd(argList, strNewFmt("--repo1-s3-region=%s", strPtr(region)));
strLstAdd(argList, strNewFmt("--repo1-s3-endpoint=%s:999", strPtr(endPoint)));
strLstAddZ(argList, "--repo1-s3-ca-path=" TLS_CERT_FAKE_PATH);
strLstAddZ(argList, "--repo1-s3-ca-file=" TLS_CERT_FAKE_PATH "/pgbackrest-test.crt");
setenv("PGBACKREST_REPO1_S3_KEY", strPtr(accessKey), true);
setenv("PGBACKREST_REPO1_S3_KEY_SECRET", strPtr(secretAccessKey), true);
setenv("PGBACKREST_REPO1_S3_TOKEN", strPtr(securityToken), true);
strLstAddZ(argList, "archive-get");
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));
TEST_ASSIGN(storage, storageRepoGet(strNew(STORAGE_TYPE_S3), false), "get S3 repo storage with options");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->bucket), strPtr(bucket), " check bucket");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->region), strPtr(region), " check region");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->host), strPtr(strNewFmt("%s.%s", strPtr(bucket), strPtr(endPoint))), " check host");
TEST_RESULT_UINT(((StorageDriverS3 *)storage->driver)->port, 999, " check port");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->accessKey), strPtr(accessKey), " check access key");
TEST_RESULT_STR(
strPtr(((StorageDriverS3 *)storage->driver)->secretAccessKey), strPtr(secretAccessKey), " check secret access key");
TEST_RESULT_STR(
strPtr(((StorageDriverS3 *)storage->driver)->securityToken), strPtr(securityToken), " check security token");
// Also add port to the host
// -------------------------------------------------------------------------------------------------------------------------
argList = strLstNew();
strLstAddZ(argList, "pgbackrest");
strLstAddZ(argList, "--stanza=db");
strLstAddZ(argList, "--repo1-type=s3");
strLstAdd(argList, strNewFmt("--repo1-path=%s", strPtr(path)));
strLstAdd(argList, strNewFmt("--repo1-s3-bucket=%s", strPtr(bucket)));
strLstAdd(argList, strNewFmt("--repo1-s3-region=%s", strPtr(region)));
strLstAdd(argList, strNewFmt("--repo1-s3-endpoint=%s:999", strPtr(endPoint)));
strLstAdd(argList, strNewFmt("--repo1-s3-host=%s:7777", strPtr(host)));
strLstAddZ(argList, "--repo1-s3-ca-path=" TLS_CERT_FAKE_PATH);
strLstAddZ(argList, "--repo1-s3-ca-file=" TLS_CERT_FAKE_PATH "/pgbackrest-test.crt");
setenv("PGBACKREST_REPO1_S3_KEY", strPtr(accessKey), true);
setenv("PGBACKREST_REPO1_S3_KEY_SECRET", strPtr(secretAccessKey), true);
setenv("PGBACKREST_REPO1_S3_TOKEN", strPtr(securityToken), true);
strLstAddZ(argList, "archive-get");
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));
TEST_ASSIGN(storage, storageRepoGet(strNew(STORAGE_TYPE_S3), false), "get S3 repo storage with options");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->bucket), strPtr(bucket), " check bucket");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->region), strPtr(region), " check region");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->host), strPtr(host), " check host");
TEST_RESULT_UINT(((StorageDriverS3 *)storage->driver)->port, 7777, " check port");
TEST_RESULT_STR(strPtr(((StorageDriverS3 *)storage->driver)->accessKey), strPtr(accessKey), " check access key");
TEST_RESULT_STR(
strPtr(((StorageDriverS3 *)storage->driver)->secretAccessKey), strPtr(secretAccessKey), " check secret access key");