mirror of
https://github.com/pgbackrest/pgbackrest.git
synced 2024-12-12 10:04:14 +02:00
Fix issue with = character in file or database names.
The manifest uses the = character as the key/value separator so = characters in the key cause parsing errors and lead to an error or segfault. Since the value must be valid JSON we can keep checking the value on the right side of the = and stop building the key when the value is valid. It's a bit hackish but it does seem to do the job without breaking the manifest format. Unsurprisingly this makes parsing about 50% slower but it's still more than fast enough. Parsing 10 million key/values takes about 6.5s for the old code and 10s for the new code. Since the value is used as JSON downstream we can reclaim most of this time by just passing the JSON value rather than making the callback reparse it. We'll save that for another commit, though.
This commit is contained in:
David Steele
GitHub
parent
63a93db6fd
commit
039d314438
@ -25,6 +25,17 @@
|
||||
|
||||
<p>Since the command has completed it is counterproductive to throw an error but still <b>warn</b> to indicate that something unusual happened.</p>
|
||||
</release-item>
|
||||
|
||||
<release-item>
|
||||
<release-item-contributor-list>
|
||||
<release-item-ideator id="brad.nicholson"/>
|
||||
<release-item-ideator id="bastian.wegge"/>
|
||||
<release-item-reviewer id="bastian.wegge"/>
|
||||
<release-item-reviewer id="cynthia.shang"/>
|
||||
</release-item-contributor-list>
|
||||
|
||||
<p>Fix issue with <id>=</id> character in file or database names.</p>
|
||||
</release-item>
|
||||
</release-bug-list>
|
||||
</release-core-list>
|
||||
|
||||
@ -8396,6 +8407,11 @@
|
||||
<contributor-id type="github">argdenis</contributor-id>
|
||||
</contributor>
|
||||
|
||||
<contributor id="bastian.wegge">
|
||||
<contributor-name-display>Bastian Wegge</contributor-name-display>
|
||||
<contributor-id type="github">bastianwegge</contributor-id>
|
||||
</contributor>
|
||||
|
||||
<contributor id="benoit.lobréau">
|
||||
<contributor-name-display>blogh</contributor-name-display>
|
||||
<contributor-id type="github">blogh</contributor-id>
|
||||
|
||||
@ -11,6 +11,7 @@ Ini Handler
|
||||
#include "common/memContext.h"
|
||||
#include "common/log.h"
|
||||
#include "common/ini.h"
|
||||
#include "common/type/json.h"
|
||||
#include "common/type/keyValue.h"
|
||||
#include "common/type/object.h"
|
||||
|
||||
@ -388,14 +389,49 @@ iniLoad(
|
||||
if (lineEqual == NULL)
|
||||
THROW_FMT(FormatError, "missing '=' in key/value at line %u: %s", lineIdx + 1, linePtr);
|
||||
|
||||
// Extract the key
|
||||
String *key = strTrim(strNewN(linePtr, (size_t)(lineEqual - linePtr)));
|
||||
// Extract the key/value. This may require some retries if the key includes an = character since this is
|
||||
// also the separator. We know the value must be valid JSON so if it isn't then add the characters up to
|
||||
// the next = to the key and try to parse the value as JSON again. If the value never becomes valid JSON
|
||||
// then an error is thrown.
|
||||
String *key;
|
||||
String *value;
|
||||
|
||||
bool retry;
|
||||
|
||||
do
|
||||
{
|
||||
retry = false;
|
||||
|
||||
// Get key/value
|
||||
key = strTrim(strNewN(linePtr, (size_t)(lineEqual - linePtr)));
|
||||
value = strTrim(strNew(lineEqual + 1));
|
||||
|
||||
// Check that the value is valid JSON
|
||||
TRY_BEGIN()
|
||||
{
|
||||
jsonToVar(value);
|
||||
}
|
||||
CATCH(JsonFormatError)
|
||||
{
|
||||
// If value is not valid JSON look for another =. If not found then nothing to retry.
|
||||
lineEqual = strstr(lineEqual + 1, "=");
|
||||
|
||||
if (lineEqual == NULL)
|
||||
THROW_FMT(FormatError, "invalid JSON value at line %u: %s", lineIdx + 1, linePtr);
|
||||
|
||||
// Try again with = in new position
|
||||
retry = true;
|
||||
}
|
||||
TRY_END();
|
||||
}
|
||||
while (retry);
|
||||
|
||||
// Key may not be zero-length
|
||||
if (strSize(key) == 0)
|
||||
THROW_FMT(FormatError, "key is zero-length at line %u: %s", lineIdx++, linePtr);
|
||||
|
||||
// Callback with the section/key/value
|
||||
callbackFunction(callbackData, section, key, strTrim(strNew(lineEqual + 1)));
|
||||
callbackFunction(callbackData, section, key, value);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -51,6 +51,14 @@ testRun(void)
|
||||
iniLoad(ioBufferReadNew(iniBuf), testIniLoadCallback, result), FormatError,
|
||||
"key/value found outside of section at line 1: key=value");
|
||||
|
||||
// -------------------------------------------------------------------------------------------------------------------------
|
||||
TEST_TITLE("invalid JSON value");
|
||||
|
||||
iniBuf = BUFSTRZ("[section]\nkey=value\n");
|
||||
|
||||
TEST_ERROR(
|
||||
iniLoad(ioBufferReadNew(iniBuf), testIniLoadCallback, result), FormatError, "invalid JSON value at line 2: key=value");
|
||||
|
||||
// Key outside of section
|
||||
// -------------------------------------------------------------------------------------------------------------------------
|
||||
iniBuf = BUFSTRZ(
|
||||
@ -78,14 +86,18 @@ testRun(void)
|
||||
"# comment\n"
|
||||
"[section1]\n"
|
||||
"key1=\"value1\"\n"
|
||||
"key2=\"value2\"\n");
|
||||
"key2=\"value2\"\n"
|
||||
"key=3==\"value3\"\n"
|
||||
"==\"=\"");
|
||||
result = strNew("");
|
||||
|
||||
TEST_RESULT_VOID(iniLoad(ioBufferReadNew(iniBuf), testIniLoadCallback, result), "load ini");
|
||||
TEST_RESULT_STR_Z(
|
||||
result,
|
||||
"section1:key1:\"value1\"\n"
|
||||
"section1:key2:\"value2\"\n",
|
||||
"section1:key2:\"value2\"\n"
|
||||
"section1:key=3=:\"value3\"\n"
|
||||
"section1:=:\"=\"\n",
|
||||
" check ini");
|
||||
|
||||
// Two sections
|
||||
@ -97,7 +109,6 @@ testRun(void)
|
||||
"key2=\"value2\"\n"
|
||||
"\n"
|
||||
"[section2]\n"
|
||||
"key1=\n"
|
||||
"\n"
|
||||
"key2=\"value2\"");
|
||||
result = strNew("");
|
||||
@ -107,7 +118,6 @@ testRun(void)
|
||||
result,
|
||||
"section1:key1:\"value1\"\n"
|
||||
"section1:key2:\"value2\"\n"
|
||||
"section2:key1:\n"
|
||||
"section2:key2:\"value2\"\n",
|
||||
" check ini");
|
||||
}
|
||||
|
||||
@ -1674,6 +1674,7 @@ testRun(void)
|
||||
#define TEST_MANIFEST_DB \
|
||||
"\n" \
|
||||
"[db]\n" \
|
||||
"=={\"db-id\":16455,\"db-last-system-id\":12168}\n" \
|
||||
"mail={\"db-id\":16456,\"db-last-system-id\":12168}\n" \
|
||||
"postgres={\"db-id\":12173,\"db-last-system-id\":12168}\n" \
|
||||
"template0={\"db-id\":12168,\"db-last-system-id\":12168}\n" \
|
||||
@ -1683,6 +1684,7 @@ testRun(void)
|
||||
#define TEST_MANIFEST_FILE \
|
||||
"\n" \
|
||||
"[target:file]\n" \
|
||||
"pg_data/=equal=more=={\"master\":true,\"mode\":\"0640\",\"size\":0,\"timestamp\":1565282120}\n" \
|
||||
"pg_data/PG_VERSION={\"checksum\":\"184473f470864e067ee3a22e64b47b0a1c356f29\",\"master\":true" \
|
||||
",\"reference\":\"20190818-084502F_20190819-084506D\",\"size\":4,\"timestamp\":1565282114}\n" \
|
||||
"pg_data/base/16384/17000={\"checksum\":\"e0101dd8ffb910c9c202ca35b5f828bcb9697bed\",\"checksum-page\":false" \
|
||||
@ -1771,6 +1773,7 @@ testRun(void)
|
||||
TEST_MANIFEST_TARGET
|
||||
"\n"
|
||||
"[db]\n"
|
||||
"=={\"db-id\":16455,\"db-last-system-id\":12168}\n"
|
||||
"mail={\"db-id\":16456,\"db-last-system-id\":12168}\n"
|
||||
"postgres={\"db-id\":12173,\"db-last-system-id\":12168}\n"
|
||||
TEST_MANIFEST_FILE
|
||||
|
||||
Loading…
Reference in New Issue
Block a user