Bugfix/cldsrv 936/list lifecycle non currents baremaster hf 9.2.36

.github/actions/setup-ci/action.yaml

@@ -27,7 +27,7 @@ runs:
         mkdir -p /tmp/coverage/${JOB_NAME}/;
     - uses: actions/setup-node@v4
       with:
-        node-version: '22'
+        node-version: '22.23.1'
         cache: 'yarn'
     - name: install typescript
       shell: bash

.github/workflows/tests.yaml

@@ -82,7 +82,7 @@ jobs:
       uses: actions/checkout@v4
     - uses: actions/setup-node@v4
       with:
-        node-version: '22'
+        node-version: '22.23.1'
         cache: yarn
     - name: install typescript
       shell: bash
@@ -111,7 +111,7 @@ jobs:
       uses: actions/checkout@v4
     - uses: actions/setup-node@v4
       with:
-        node-version: '22'
+        node-version: '22.23.1'
         cache: yarn
     - name: install typescript
       shell: bash

lib/api/apiUtils/object/lifecycle.js

@@ -1,4 +1,4 @@
-const { versioning } = require('arsenal');
+const { versioning, errorInstances } = require('arsenal');
 const versionIdUtils = versioning.VersionID;
 
 const { lifecycleListing } = require('../../../../constants');
@@ -70,6 +70,19 @@ function _encodeVersionId(vid) {
     return versionId;
 }
 
+// A 'null' or absent marker means "no marker". Any other value is decoded,
+// returning InvalidArgument if it is not a valid version id.
+function decodeVersionIdMarker(vid) {
+    if (!vid || vid === 'null') {
+        return undefined;
+    }
+    const decoded = versionIdUtils.decode(vid);
+    if (decoded instanceof Error) {
+        return errorInstances.InvalidArgument.customizeDescription('Invalid version id marker specified');
+    }
+    return decoded;
+}
+
 function processNonCurrents(bucketName, listParams, list) {
     const nextVersionIdMarker = _encodeVersionId(list.NextVersionIdMarker);
     const versionIdMarker = _encodeVersionId(listParams.versionIdMarker);
@@ -187,4 +200,5 @@ module.exports = {
     processOrphans,
     getLocationConstraintErrorMessage,
     validateMaxScannedEntries,
+    decodeVersionIdMarker,
 };

lib/api/backbeat/listLifecycleNonCurrents.js

@@ -1,12 +1,11 @@
-const { errors, errorInstances, versioning } = require('arsenal');
+const { errors, errorInstances } = require('arsenal');
 const constants = require('../../../constants');
 const services = require('../../services');
 const { standardMetadataValidateBucket } = require('../../metadata/metadataUtils');
 const { pushMetric } = require('../../utapi/utilities');
-const versionIdUtils = versioning.VersionID;
 const monitoring = require('../../utilities/monitoringHandler');
 const { getLocationConstraintErrorMessage, processNonCurrents,
-    validateMaxScannedEntries } = require('../apiUtils/object/lifecycle');
+    validateMaxScannedEntries, decodeVersionIdMarker } = require('../apiUtils/object/lifecycle');
 const { config } = require('../../Config');
 
 function handleResult(listParams, requestMaxKeys, authInfo,
@@ -80,8 +79,13 @@ function listLifecycleNonCurrents(authInfo, locationConstraints, request, log, c
         maxScannedLifecycleListingEntries,
     };
 
-    listParams.versionIdMarker = params['version-id-marker'] ?
-        versionIdUtils.decode(params['version-id-marker']) : undefined;
+    const versionIdMarker = decodeVersionIdMarker(params['version-id-marker']);
+    if (versionIdMarker instanceof Error) {
+        log.debug('invalid version id marker', { versionIdMarker: params['version-id-marker'] });
+        monitoring.promMetrics('GET', bucketName, 400, 'listLifecycleNonCurrents');
+        return callback(versionIdMarker);
+    }
+    listParams.versionIdMarker = versionIdMarker;
 
     return standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
         if (err) {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@zenko/cloudserver",
-  "version": "9.2.36",
+  "version": "9.2.36-1",
   "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
   "main": "index.js",
   "engines": {

tests/functional/backbeat/listLifecycleNonCurrents.js

@@ -255,6 +255,26 @@ describe('listLifecycleNonCurrents', () => {
         });
     });
 
+    it('should return InvalidArgument error if version-id-marker is malformed', done => {
+        makeBackbeatRequest({
+            method: 'GET',
+            bucket: testBucket,
+            queryObj: {
+                'list-type': 'noncurrent',
+                'key-marker': 'key1',
+                'version-id-marker': 'notavalidversionid',
+            },
+            authCredentials: credentials,
+        }, err => {
+            // A non-null, undecodable marker must be rejected with a 400
+            // rather than reaching _encodeVersionId and crashing the worker.
+            assert(err);
+            assert.strictEqual(err.code, 'InvalidArgument');
+            assert.strictEqual(err.statusCode, 400);
+            return done();
+        });
+    });
+
     it('should return error if bucket not versioned', done => {
         makeBackbeatRequest({
             method: 'GET',
@@ -614,4 +634,43 @@ describe('listLifecycleNonCurrents', () => {
             return done();
         });
     });
+
+    // Regression test: a scan-limit truncation that stops on a "bare master"
+    // (a pre-versioning object with no internal versionId) returns a
+    // version-id-marker equals to 'null'. On the following listing, 
+    // CloudServer should treat it as "no marker" instead of decoding it. 
+    // Decoding it was throwing and crashing the worker.
+    it('should treat a version-id-marker of "null" as no marker without crashing', done => {
+        makeBackbeatRequest({
+            method: 'GET',
+            bucket: testBucket,
+            queryObj: {
+                'list-type': 'noncurrent',
+                'before-date': date,
+                'key-marker': 'key1',
+                'version-id-marker': 'null',
+            },
+            authCredentials: credentials,
+        }, (err, response) => {
+            assert.ifError(err);
+            // Before the fix this request crashed the worker; assert it succeeds.
+            assert.strictEqual(response.statusCode, 200);
+            const data = JSON.parse(response.body);
+
+            assert.strictEqual(data.KeyMarker, 'key1');
+            // 'null' is echoed back unchanged and treated as no marker, so the
+            // listing resumes after key1 and returns key2's noncurrent versions.
+            assert.strictEqual(data.IsTruncated, false);
+            assert(!data.NextKeyMarker);
+
+            const contents = data.Contents;
+            checkContents(contents);
+            assert.strictEqual(contents.length, expectedKey2VersionIds.length);
+            contents.forEach((c, i) => {
+                assert.strictEqual(c.Key, 'key2');
+                assert.strictEqual(c.VersionId, expectedKey2VersionIds[i]);
+            });
+            return done();
+        });
+    });
 });

tests/unit/api/apiUtils/lifecycle.js

@@ -1,7 +1,13 @@
 const assert = require('assert');
-const { validateMaxScannedEntries } =
+const { versioning } = require('arsenal');
+const { validateMaxScannedEntries, decodeVersionIdMarker } =
       require('../../../../lib/api/apiUtils/object/lifecycle');
 
+const versionIdUtils = versioning.VersionID;
+// A valid encoded marker that round-trips to this internal version id.
+const validInternalVid = '98765432109876999999PARIS00';
+const validEncodedMarker = versionIdUtils.encode(validInternalVid);
+
 const tests = [
     {
         it: 'should return config value if no query params set',
@@ -48,3 +54,23 @@ describe('validateMaxScannedEntries helper', () => {
         });
     });
 });
+
+describe('decodeVersionIdMarker helper', () => {
+    ['null', '', undefined].forEach(vid => {
+        it(`should treat ${JSON.stringify(vid)} as no marker`, () => {
+            assert.strictEqual(decodeVersionIdMarker(vid), undefined);
+        });
+    });
+
+    it('should decode a valid encoded marker', () => {
+        assert.strictEqual(decodeVersionIdMarker(validEncodedMarker), validInternalVid);
+    });
+
+    it('should return InvalidArgument when decode returns an Error value', () => {
+        // a malformed (non-null) marker that decode rejects by returning an Error
+        const result = decodeVersionIdMarker('@@@bad@@@');
+        assert(result instanceof Error);
+        assert.strictEqual(result.message, 'InvalidArgument');
+        assert.strictEqual(result.description, 'Invalid version id marker specified');
+    });
+});