1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2026-10-23 21:28:56 +03:00

Fix pagination of S3 version listing.

Listing with a repository target time uses the ListObjectVersions API but pagination read NextContinuationToken from the response, which is only returned by ListObjectsV2. A version listing truncated at the maximum key count failed with "unable to find child 'NextContinuationToken':0 in node 'ListVersionsResult'", so a repository with more than a thousand versions under a prefix could not be read at a target time.

Paginate the version listing with NextKeyMarker and NextVersionIdMarker instead. NextVersionIdMarker is missing or empty when the listing is truncated at a common prefix, in which case the next page starts at the key after the key marker, which is what is needed.

Build the query for each page from the base query so a marker set for one page is not reused for the next.

The mock response used to test version listing pagination returned a continuation token rather than markers, so the error was not caught.
This commit is contained in:
David Steele
2026-09-15 10:33:08 +03:00
committed by GitHub
parent ce590e9d9b
commit 917f27c7b3
3 changed files with 91 additions and 16 deletions

View File

@@ -12,6 +12,20 @@
<p>Fix <cmd>annotate</cmd> command on a remote encrypted repository.</p>
</release-item>
<release-item>
<github-issue id="2854"/>
<github-pull-request id="2855"/>
<release-item-contributor-list>
<release-item-ideator id="guruguruguru"/>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="douglas.j.hunley"/>
<release-item-reviewer id="guruguruguru"/>
</release-item-contributor-list>
<p>Fix pagination of <proper>S3</proper> version listing.</p>
</release-item>
<release-item>
<github-issue id="2840"/>
<github-pull-request id="2848"/>

View File

@@ -49,8 +49,10 @@ S3 query tokens
STRING_STATIC(S3_QUERY_CONTINUATION_TOKEN_STR, "continuation-token");
STRING_STATIC(S3_QUERY_DELETE_STR, "delete");
STRING_STATIC(S3_QUERY_DELIMITER_STR, "delimiter");
STRING_STATIC(S3_QUERY_KEY_MARKER_STR, "key-marker");
STRING_STATIC(S3_QUERY_LIST_TYPE_STR, "list-type");
STRING_STATIC(S3_QUERY_PREFIX_STR, "prefix");
STRING_STATIC(S3_QUERY_VERSION_ID_MARKER_STR, "version-id-marker");
STRING_STATIC(S3_QUERY_VERSIONS_STR, "versions");
STRING_STATIC(S3_QUERY_VALUE_LIST_TYPE_2_STR, "2");
@@ -68,6 +70,9 @@ STRING_STATIC(S3_XML_TAG_KEY_STR, "Key");
STRING_STATIC(S3_XML_TAG_LAST_MODIFIED_STR, "LastModified");
#define S3_XML_TAG_NEXT_CONTINUATION_TOKEN "NextContinuationToken"
STRING_STATIC(S3_XML_TAG_NEXT_CONTINUATION_TOKEN_STR, S3_XML_TAG_NEXT_CONTINUATION_TOKEN);
#define S3_XML_TAG_NEXT_KEY_MARKER "NextKeyMarker"
STRING_STATIC(S3_XML_TAG_NEXT_KEY_MARKER_STR, S3_XML_TAG_NEXT_KEY_MARKER);
STRING_STATIC(S3_XML_TAG_NEXT_VERSION_ID_MARKER_STR, "NextVersionIdMarker");
STRING_STATIC(S3_XML_TAG_OBJECT_STR, "Object");
STRING_STATIC(S3_XML_TAG_PREFIX_STR, "Prefix");
STRING_STATIC(S3_XML_TAG_QUIET_STR, "Quiet");
@@ -839,16 +844,42 @@ storageS3ListInternal(
// If list is truncated then send an async request to get more data
if (strEq(xmlNodeContent(xmlNodeChild(xmlRoot, S3_XML_TAG_IS_TRUNCATED_STR, true)), TRUE_STR))
{
const String *const nextContinuationToken = xmlNodeContent(
xmlNodeChild(xmlRoot, S3_XML_TAG_NEXT_CONTINUATION_TOKEN_STR, true));
CHECK(FormatError, !strEmpty(nextContinuationToken), S3_XML_TAG_NEXT_CONTINUATION_TOKEN " may not be empty");
// Build the query for the next page from the base query since the markers are set per page
HttpQuery *const queryNext = httpQueryDupP(query);
httpQueryPut(query, S3_QUERY_CONTINUATION_TOKEN_STR, nextContinuationToken);
// Version listing is paginated with key and version id markers
if (targetTime != 0)
{
const String *const nextKeyMarker = xmlNodeContent(
xmlNodeChild(xmlRoot, S3_XML_TAG_NEXT_KEY_MARKER_STR, true));
CHECK(FormatError, !strEmpty(nextKeyMarker), S3_XML_TAG_NEXT_KEY_MARKER " may not be empty");
httpQueryAdd(queryNext, S3_QUERY_KEY_MARKER_STR, nextKeyMarker);
// The version id marker may be missing or empty when the next result is a common prefix. In that case the
// next page starts at the key after the key marker, which is what is needed.
const String *const nextVersionIdMarker = xmlNodeContent(
xmlNodeChild(xmlRoot, S3_XML_TAG_NEXT_VERSION_ID_MARKER_STR, false));
if (nextVersionIdMarker != NULL && !strEmpty(nextVersionIdMarker))
httpQueryAdd(queryNext, S3_QUERY_VERSION_ID_MARKER_STR, nextVersionIdMarker);
}
// Else object listing is paginated with a continuation token
else
{
const String *const nextContinuationToken = xmlNodeContent(
xmlNodeChild(xmlRoot, S3_XML_TAG_NEXT_CONTINUATION_TOKEN_STR, true));
CHECK(
FormatError, !strEmpty(nextContinuationToken),
S3_XML_TAG_NEXT_CONTINUATION_TOKEN " may not be empty");
httpQueryAdd(queryNext, S3_QUERY_CONTINUATION_TOKEN_STR, nextContinuationToken);
}
// Store request in the outer temp context
MEM_CONTEXT_PRIOR_BEGIN()
{
request = storageS3RequestAsyncP(this, HTTP_VERB_GET_STR, FSLASH_STR, .query = query);
request = storageS3RequestAsyncP(this, HTTP_VERB_GET_STR, FSLASH_STR, .query = queryNext);
}
MEM_CONTEXT_PRIOR_END();
}

View File

@@ -1900,14 +1900,16 @@ testRun(void)
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("list with time limit");
// Truncated in the middle of the versions for a key so both markers are returned
testRequestP(service, s3, HTTP_VERB_GET, "/?delimiter=%2F&prefix=path%2Fto%2F&versions=", .requesterPays = true);
testResponseP(
service,
.content =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
"<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
"<ListVersionsResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
" <IsTruncated>true</IsTruncated>"
" <NextContinuationToken>1ueG</NextContinuationToken>"
" <NextKeyMarker>path/to/test_file</NextKeyMarker>"
" <NextVersionIdMarker>aaaa</NextVersionIdMarker>"
" <DeleteMarker>"
" <Key>path/to/test_file</Key>"
" <LastModified>2024-08-04T02:54:10.000Z</LastModified>"
@@ -1916,17 +1918,21 @@ testRun(void)
" <Key>path/to/test_file</Key>"
" <LastModified>2024-08-04T02:54:10.000Z</LastModified>"
" </Version>"
"</ListBucketResult>");
"</ListVersionsResult>");
// Truncated at a key boundary with an empty version id marker
testRequestP(
service, s3, HTTP_VERB_GET, "/?continuation-token=1ueG&delimiter=%2F&prefix=path%2Fto%2F&versions=",
service, s3, HTTP_VERB_GET,
"/?delimiter=%2F&key-marker=path%2Fto%2Ftest_file&prefix=path%2Fto%2F&version-id-marker=aaaa&versions=",
.requesterPays = true);
testResponseP(
service,
.content =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
"<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
" <IsTruncated>false</IsTruncated>"
"<ListVersionsResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
" <IsTruncated>true</IsTruncated>"
" <NextKeyMarker>path/to/test_file2</NextKeyMarker>"
" <NextVersionIdMarker></NextVersionIdMarker>"
" <Version>"
" <Key>path/to/test_file</Key>"
" <LastModified>2024-08-04T02:54:09.000Z</LastModified>"
@@ -1937,14 +1943,38 @@ testRun(void)
" <Key>path/to/test_file</Key>"
" <LastModified>2024-08-04T02:54:09.000Z</LastModified>"
" </Version>"
"</ListVersionsResult>");
// Truncated at a common prefix so no version id marker is returned
testRequestP(
service, s3, HTTP_VERB_GET,
"/?delimiter=%2F&key-marker=path%2Fto%2Ftest_file2&prefix=path%2Fto%2F&versions=", .requesterPays = true);
testResponseP(
service,
.content =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
"<ListVersionsResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
" <IsTruncated>true</IsTruncated>"
" <NextKeyMarker>path/to/test_path/</NextKeyMarker>"
" <Version>"
" <Key>path/to/test_file2</Key>"
" <LastModified>2024-08-04T02:54:10.000Z</LastModified>"
" </Version>"
"</ListVersionsResult>");
testRequestP(
service, s3, HTTP_VERB_GET,
"/?delimiter=%2F&key-marker=path%2Fto%2Ftest_path%2F&prefix=path%2Fto%2F&versions=", .requesterPays = true);
testResponseP(
service,
.content =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
"<ListVersionsResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
" <IsTruncated>false</IsTruncated>"
" <CommonPrefixes>"
" <Prefix>path/to/test_path/</Prefix>"
" </CommonPrefixes>"
"</ListBucketResult>");
"</ListVersionsResult>");
TEST_STORAGE_LIST(
s3, "/path/to",
@@ -1960,7 +1990,7 @@ testRun(void)
service,
.content =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
"<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
"<ListVersionsResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
" <IsTruncated>false</IsTruncated>"
" <Version>"
" <Key>path/3/test_file</Key>"
@@ -1975,7 +2005,7 @@ testRun(void)
" <CommonPrefixes>"
" <Prefix>path/3/test_path/</Prefix>"
" </CommonPrefixes>"
"</ListBucketResult>");
"</ListVersionsResult>");
testRequestP(service, s3, HTTP_VERB_GET, "/path/3/test_file?versionId=bbbb", .requesterPays = true);
testResponseP(service, .content = "123456");
@@ -1991,7 +2021,7 @@ testRun(void)
service,
.content =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
"<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
"<ListVersionsResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
" <IsTruncated>false</IsTruncated>"
" <Version>"
" <Key>missing_file</Key>"
@@ -2004,7 +2034,7 @@ testRun(void)
" <CommonPrefixes>"
" <Prefix>test_path/</Prefix>"
" </CommonPrefixes>"
"</ListBucketResult>");
"</ListVersionsResult>");
TEST_RESULT_PTR(
storageGetP(storageNewReadP(s3, STRDEF("missing_file"), .ignoreMissing = true)), NULL, "missing file");