S3UTILS-195 verifyBucketSproxydKeys.js crashes on empty object without location

Author: nicolas2bert

VerifyBucketSproxydKeys/validateLocations.js

@@ -0,0 +1,31 @@
+const { Logger } = require('werelogs');
+
+const log = new Logger('s3utils:validateLocations');
+
+/**
+ * Validates that locations array is valid for objects with non-zero content-length
+ * @param {string} objectUrl - The S3 object URL for logging
+ * @param {Array} locations - The locations array from metadata
+ * @param {Object} status - Status object to update counters
+ * @param {Object} findDuplicateSproxydKeys - Object with skipVersion method
+ * @returns {boolean} - true if valid, false if invalid
+ */
+function validateLocations(objectUrl, locations, status, findDuplicateSproxydKeys) {
+    // Handle broken metadata where locations is invalid or empty for non-zero content-length objects
+    if (!Array.isArray(locations) || locations.length === 0) {
+        log.error('object with non-zero content-length has invalid or empty location data', {
+            objectUrl,
+            locations,
+            locationType: typeof locations,
+        });
+        // eslint-disable-next-line no-param-reassign
+        status.objectsScanned += 1;
+        // eslint-disable-next-line no-param-reassign
+        status.objectsWithBrokenMetadata += 1;
+        findDuplicateSproxydKeys.skipVersion();
+        return false;
+    }
+    return true;
+}
+
+module.exports = validateLocations;

tests/unit/verifyBucketSproxydKeys.js

@@ -1,6 +1,7 @@
 const getObjectURL = require('../../VerifyBucketSproxydKeys/getObjectURL');
 const getBucketdURL = require('../../VerifyBucketSproxydKeys/getBucketdURL');
 const FindDuplicateSproxydKeys = require('../../VerifyBucketSproxydKeys/FindDuplicateSproxydKeys');
+const validateLocations = require('../../VerifyBucketSproxydKeys/validateLocations');
 
 describe('verifyBucketSproxydKeys', () => {
     test('getObjectURL', () => {
@@ -77,4 +78,93 @@ describe('verifyBucketSproxydKeys', () => {
         expect(finder.sproxydKeys).toEqual({ k7: 'obj9' });
         expect(finder.versionsWindow).toEqual({ 11: ['k7'], 12: ['k7'] });
     });
+
+    describe('validateLocations', () => {
+        const key1 = {
+            key: '8df148c188a7369ef6b632b08e9a2a867c065761',
+            size: 2097,
+            start: 0,
+            dataStoreName: 'us-east-1',
+            dataStoreType: 'scality',
+            dataStoreETag: '1:35bf6d36c0c721deceda5d15fa642a18',
+        };
+        const key2 = {
+            key: '8df148c188a7369ef6b632b08e9a2a867c065762',
+            size: 2098,
+            start: 0,
+            dataStoreName: 'us-east-1',
+            dataStoreType: 'scality',
+            dataStoreETag: '1:35bf6d36c0c721deceda5d15fa642a19',
+        };
+        let status;
+        let findDuplicateSproxydKeys;
+
+        beforeEach(() => {
+            status = { objectsScanned: 0, objectsWithBrokenMetadata: 0 };
+            findDuplicateSproxydKeys = { skipVersion: jest.fn() };
+        });
+
+        test('should return true for valid locations array', () => {
+            const result = validateLocations('s3://bucket/key', [key1], status, findDuplicateSproxydKeys);
+
+            expect(result).toEqual(true);
+            expect(status.objectsScanned).toEqual(0);
+            expect(status.objectsWithBrokenMetadata).toEqual(0);
+            expect(findDuplicateSproxydKeys.skipVersion).not.toHaveBeenCalled();
+        });
+
+        test('should return true for multiple valid locations', () => {
+            const result = validateLocations('s3://bucket/valid-key', [key1, key2], status, findDuplicateSproxydKeys);
+
+            expect(result).toEqual(true);
+            expect(status.objectsScanned).toEqual(0);
+            expect(status.objectsWithBrokenMetadata).toEqual(0);
+            expect(findDuplicateSproxydKeys.skipVersion).not.toHaveBeenCalled();
+        });
+
+        test('should return false for undefined locations', () => {
+            const result = validateLocations('s3://bucket/broken-key', undefined, status, findDuplicateSproxydKeys);
+
+            expect(result).toEqual(false);
+            expect(status.objectsScanned).toEqual(1);
+            expect(status.objectsWithBrokenMetadata).toEqual(1);
+            expect(findDuplicateSproxydKeys.skipVersion).toHaveBeenCalledTimes(1);
+        });
+
+        test('should return false for null locations', () => {
+            const result = validateLocations('s3://bucket/null-key', null, status, findDuplicateSproxydKeys);
+
+            expect(result).toEqual(false);
+            expect(status.objectsScanned).toEqual(1);
+            expect(status.objectsWithBrokenMetadata).toEqual(1);
+            expect(findDuplicateSproxydKeys.skipVersion).toHaveBeenCalledTimes(1);
+        });
+
+        test('should return false for empty array locations', () => {
+            const result = validateLocations('s3://bucket/empty-key', [], status, findDuplicateSproxydKeys);
+
+            expect(result).toEqual(false);
+            expect(status.objectsScanned).toEqual(1);
+            expect(status.objectsWithBrokenMetadata).toEqual(1);
+            expect(findDuplicateSproxydKeys.skipVersion).toHaveBeenCalledTimes(1);
+        });
+
+        test('should return false for non-array locations', () => {
+            const result = validateLocations('s3://bucket/string-key', 'not-an-array', status, findDuplicateSproxydKeys);
+
+            expect(result).toEqual(false);
+            expect(status.objectsScanned).toEqual(1);
+            expect(status.objectsWithBrokenMetadata).toEqual(1);
+            expect(findDuplicateSproxydKeys.skipVersion).toHaveBeenCalledTimes(1);
+        });
+
+        test('should return false for object instead of array', () => {
+            const result = validateLocations('s3://bucket/object-key', { key: 'sproxyd-key' }, status, findDuplicateSproxydKeys);
+
+            expect(result).toEqual(false);
+            expect(status.objectsScanned).toEqual(1);
+            expect(status.objectsWithBrokenMetadata).toEqual(1);
+            expect(findDuplicateSproxydKeys.skipVersion).toHaveBeenCalledTimes(1);
+        });
+    });
 });

verifyBucketSproxydKeys.js

@@ -14,6 +14,7 @@ const { Logger } = require('werelogs');
 const getObjectURL = require('./VerifyBucketSproxydKeys/getObjectURL');
 const getBucketdURL = require('./VerifyBucketSproxydKeys/getBucketdURL');
 const FindDuplicateSproxydKeys = require('./VerifyBucketSproxydKeys/FindDuplicateSproxydKeys');
+const validateLocations = require('./VerifyBucketSproxydKeys/validateLocations');
 const BlockDigestsStream = require('./CompareRaftMembers/BlockDigestsStream');
 const BlockDigestsStorage = require('./CompareRaftMembers/BlockDigestsStorage');
 
@@ -141,6 +142,7 @@ const status = {
     objectsWithDupKeys: 0,
     objectsWithEmptyMetadata: 0,
     objectsWithDupVersionIds: 0,
+    objectsWithBrokenMetadata: 0,
     objectsErrors: 0,
     bucketInProgress: null,
     KeyMarker: '',
@@ -196,6 +198,7 @@ function logProgress(message) {
         haveDupKeys: status.objectsWithDupKeys,
         haveEmptyMetadata: status.objectsWithEmptyMetadata,
         haveDupVersionIds: status.objectsWithDupVersionIds,
+        haveBrokenMetadata: status.objectsWithBrokenMetadata,
         errors: status.objectErrors,
         url: getObjectURL(status.bucketInProgress, status.KeyMarker),
     });
@@ -299,6 +302,10 @@ function fetchObjectLocations(bucket, objectKey, cb) {
 }
 
 function checkSproxydKeys(objectUrl, locations, cb) {
+    if (!validateLocations(objectUrl, locations, status, findDuplicateSproxydKeys)) {
+        return process.nextTick(cb);
+    }
+
     let keyError = false;
     let keyMissing = false;
     let dupKey = false;
@@ -649,6 +656,9 @@ function main() {
             if (status.objectsWithEmptyMetadata) {
                 process.exit(104);
             }
+            if (status.objectsWithBrokenMetadata) {
+                process.exit(105);
+            }
             if (status.objectsErrors) {
                 process.exit(103);
             }