handle \n and \ properly in the journals

Signed-off-by: Utkarsh Srivastava <[email protected]>

Author: tangledbytes

src/sdk/glacier.js

@@ -167,6 +167,32 @@ class Glacier {
         return restore_status.state === Glacier.RESTORE_STATUS_CAN_RESTORE;
     }
 
+    /**
+     * encode_log takes in data intended for the backend and encodes
+     * it.
+     * 
+     * This method must be overwritten for all the backends if they need
+     * different encodings for their logs.
+     * @param {string} data 
+     * @returns {string}
+     */
+    encode_log(data) {
+        return data;
+    }
+
+    /**
+     * decode_log takes in data intended for the backend and decodes
+     * it.
+     * 
+     * This method must be overwritten for all the backends if they need
+     * different encodings for their logs.
+     * @param {string} data 
+     * @returns {string}
+     */
+    decode_log(data) {
+        return data;
+    }
+
     /**
      * get_restore_status returns status of the object at the given
      * file_path

src/sdk/glacier_tapecloud.js

@@ -190,13 +190,21 @@ class TapeCloudUtils {
 }
 
 class TapeCloudGlacier extends Glacier {
+    static LOG_DELIM = ' -- ';
+
     async migrate(fs_context, log_file, failure_recorder) {
         dbg.log2('TapeCloudGlacier.migrate starting for', log_file);
 
-        const file = new LogFile(fs_context, log_file, '\n-- ');
+        // Wrap failure recorder to make sure we correctly encode the entries
+        // before appending them to the failure log
+        const encoded_failure_recorder = async failure => failure_recorder(this.encode_log(failure));
+
+        const file = new LogFile(fs_context, log_file);
 
         try {
             await file.collect_and_process(async (entry, batch_recorder) => {
+                entry = this.decode_log(entry);
+
                 let should_migrate = true;
                 try {
                     should_migrate = await this.should_migrate(fs_context, entry);
@@ -214,7 +222,7 @@ class TapeCloudGlacier extends Glacier {
                     // Can't really do anything if this fails - provider
                     // needs to make sure that appropriate error handling
                     // is being done there
-                    await failure_recorder(entry);
+                    await encoded_failure_recorder(entry);
                     return;
                 }
 
@@ -224,13 +232,13 @@ class TapeCloudGlacier extends Glacier {
                 // Can't really do anything if this fails - provider
                 // needs to make sure that appropriate error handling
                 // is being done there
-                await batch_recorder(entry);
+                await batch_recorder(this.encode_log(entry));
             },
             async batch => {
                 // This will throw error only if our eeadm error handler
                 // panics as well and at that point it's okay to
                 // not handle the error and rather keep the log file around
-                await this._migrate(batch, failure_recorder);
+                await this._migrate(batch, encoded_failure_recorder);
             });
 
             return true;
@@ -243,9 +251,15 @@ class TapeCloudGlacier extends Glacier {
     async restore(fs_context, log_file, failure_recorder) {
         dbg.log2('TapeCloudGlacier.restore starting for', log_file);
 
-        const file = new LogFile(fs_context, log_file, '\n-- ');
+        // Wrap failure recorder to make sure we correctly encode the entries
+        // before appending them to the failure log
+        const encoded_failure_recorder = async failure => failure_recorder(this.encode_log(failure));
+
+        const file = new LogFile(fs_context, log_file);
         try {
             await file.collect_and_process(async (entry, batch_recorder) => {
+                entry = this.decode_log(entry);
+
                 try {
                     const should_restore = await Glacier.should_restore(fs_context, entry);
                     if (!should_restore) {
@@ -254,7 +268,7 @@ class TapeCloudGlacier extends Glacier {
                     }
 
                     // Add entry to the tempwal
-                    await batch_recorder(entry);
+                    await batch_recorder(this.encode_log(entry));
                 } catch (error) {
                     if (error.code === 'ENOENT') {
                         // Skip this file
@@ -265,17 +279,19 @@ class TapeCloudGlacier extends Glacier {
                         'adding log entry', entry,
                         'to failure recorder due to error', error,
                     );
-                    await failure_recorder(entry);
+                    await encoded_failure_recorder(entry);
                 }
             },
             async batch => {
                 const success = await this._recall(
                     batch,
                     async entry_path => {
+                        entry_path = this.decode_log(entry_path);
                         dbg.log2('TapeCloudGlacier.restore.partial_failure - entry:', entry_path);
-                        await failure_recorder(entry_path);
+                        await encoded_failure_recorder(entry_path);
                     },
                     async entry_path => {
+                        entry_path = this.decode_log(entry_path);
                         dbg.log2('TapeCloudGlacier.restore.partial_success - entry:', entry_path);
                         await this._finalize_restore(fs_context, entry_path);
                     }
@@ -286,6 +302,7 @@ class TapeCloudGlacier extends Glacier {
                 if (success) {
                     const batch_file = new LogFile(fs_context, batch);
                     await batch_file.collect_and_process(async (entry_path, batch_recorder) => {
+                        entry_path = this.decode_log(entry_path);
                         dbg.log2('TapeCloudGlacier.restore.batch - entry:', entry_path);
                         await this._finalize_restore(fs_context, entry_path);
                     });
@@ -311,6 +328,26 @@ class TapeCloudGlacier extends Glacier {
         return result.toLowerCase().trim() === 'true';
     }
 
+    /**
+     * 
+     * @param {string} data 
+     * @returns 
+     */
+    encode_log(data) {
+        const encoded = data.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
+        return `${TapeCloudGlacier.LOG_DELIM}${encoded}`;
+    }
+
+    /**
+     * 
+     * @param {string} data 
+     * @returns 
+     */
+    decode_log(data) {
+        return data.substring(TapeCloudGlacier.LOG_DELIM.length).replace(/\\n/g, '\n').replace(/\\\\/g, '\\');
+    }
+
+
     // ============= PRIVATE FUNCTIONS =============
 
     /**

src/sdk/namespace_fs.js

@@ -3687,13 +3687,13 @@ class NamespaceFS {
     async append_to_migrate_wal(entry) {
         if (!config.NSFS_GLACIER_LOGS_ENABLED) return;
 
-        await NamespaceFS.migrate_wal.append(`-- ${entry}`);
+        await NamespaceFS.migrate_wal.append(Glacier.getBackend().encode_log(entry));
     }
 
     async append_to_restore_wal(entry) {
         if (!config.NSFS_GLACIER_LOGS_ENABLED) return;
 
-        await NamespaceFS.restore_wal.append(`-- ${entry}`);
+        await NamespaceFS.restore_wal.append(Glacier.getBackend().encode_log(entry));
     }
 
     static get migrate_wal() {

src/test/unit_tests/nsfs/test_nsfs_glacier_backend.js

@@ -151,6 +151,8 @@ mocha.describe('nsfs_glacier', function() {
 	mocha.describe('nsfs_glacier_tapecloud', async function() {
         const upload_key = 'upload_key_1';
         const restore_key = 'restore_key_1';
+        const restore_key_spcl_char_1 = 'restore_key_2_\n';
+        const restore_key_spcl_char_2 = 'restore_key_2_\n_2';
         const xattr = { key: 'value', key2: 'value2' };
         xattr[s3_utils.XATTR_SORT_SYMBOL] = true;
 
@@ -267,10 +269,140 @@ mocha.describe('nsfs_glacier', function() {
             failure_backend._process_expired = async () => { /**noop*/ };
             failure_backend._recall = async (_file, failure_recorder, success_recorder) => {
                 // This unintentionally also replicates duplicate entries in WAL
-                await failure_recorder(failed_file_path);
+                await failure_recorder(failure_backend.encode_log(failed_file_path));
 
                 // This unintentionally also replicates duplicate entries in WAL
-                await success_recorder(success_file_path);
+                await success_recorder(failure_backend.encode_log(success_file_path));
+
+                return false;
+            };
+
+            const upload_res_1 = await glacier_ns.upload_object(failed_params, dummy_object_sdk);
+            console.log('upload_object response', inspect(upload_res_1));
+
+            const upload_res_2 = await glacier_ns.upload_object(success_params, dummy_object_sdk);
+            console.log('upload_object response', inspect(upload_res_2));
+
+            const restore_res_1 = await glacier_ns.restore_object(failed_params, dummy_object_sdk);
+            assert(restore_res_1);
+
+            const restore_res_2 = await glacier_ns.restore_object(success_params, dummy_object_sdk);
+            assert(restore_res_2);
+
+            const fs_context = glacier_ns.prepare_fs_context(dummy_object_sdk);
+
+            // Issue restore
+            await NamespaceFS.restore_wal._process(async file => {
+                await failure_backend.restore(fs_context, file, async () => { /*noop*/ });
+
+                // Don't delete the file
+                return false;
+            });
+
+            // Ensure success object is restored
+            const success_md = await glacier_ns.read_object_md(success_params, dummy_object_sdk);
+
+            assert(!success_md.restore_status.ongoing);
+
+            const expected_expiry = Glacier.generate_expiry(new Date(), success_params.days, '', config.NSFS_GLACIER_EXPIRY_TZ);
+            assert(expected_expiry.getTime() >= success_md.restore_status.expiry_time.getTime());
+            assert(now <= success_md.restore_status.expiry_time.getTime());
+
+            // Ensure failed object is NOT restored
+            const failure_stats = await nb_native().fs.stat(
+                fs_context,
+                failed_file_path,
+            );
+
+            assert(!failure_stats.xattr[Glacier.XATTR_RESTORE_EXPIRY] || failure_stats.xattr[Glacier.XATTR_RESTORE_EXPIRY] === '');
+            assert(failure_stats.xattr[Glacier.XATTR_RESTORE_REQUEST]);
+        });
+
+        mocha.it('restore-object should successfully restore objects with special characters', async function() {
+            const now = Date.now();
+            const data = crypto.randomBytes(100);
+            const all_params = [
+                {
+                    bucket: upload_bkt,
+                    key: restore_key_spcl_char_1,
+                    storage_class: s3_utils.STORAGE_CLASS_GLACIER,
+                    xattr,
+                    days: 1,
+                    source_stream: buffer_utils.buffer_to_read_stream(data)
+                },
+                {
+                    bucket: upload_bkt,
+                    key: restore_key_spcl_char_2,
+                    storage_class: s3_utils.STORAGE_CLASS_GLACIER,
+                    xattr,
+                    days: 1,
+                    source_stream: buffer_utils.buffer_to_read_stream(data)
+                }
+            ];
+
+            for (const params of all_params) {
+                const upload_res = await glacier_ns.upload_object(params, dummy_object_sdk);
+                console.log('upload_object response', inspect(upload_res));
+
+                const restore_res = await glacier_ns.restore_object(params, dummy_object_sdk);
+                assert(restore_res);
+
+                // Issue restore
+                await NamespaceFS.restore_wal._process(async file => {
+                    const fs_context = glacier_ns.prepare_fs_context(dummy_object_sdk);
+                    await backend.restore(fs_context, file);
+
+                    // Don't delete the file
+                    return false;
+                });
+
+                // Ensure object is restored
+                const md = await glacier_ns.read_object_md(params, dummy_object_sdk);
+
+                assert(!md.restore_status.ongoing);
+
+                const expected_expiry = Glacier.generate_expiry(new Date(), params.days, '', config.NSFS_GLACIER_EXPIRY_TZ);
+                assert(expected_expiry.getTime() >= md.restore_status.expiry_time.getTime());
+                assert(now <= md.restore_status.expiry_time.getTime());
+            }
+        });
+
+        mocha.it('restore-object should not restore failed item with special characters', async function() {
+            const now = Date.now();
+            const data = crypto.randomBytes(100);
+            const failed_restore_key = `${restore_key_spcl_char_1}_failured`;
+            const success_restore_key = `${restore_key_spcl_char_1}_success`;
+
+            const failed_params = {
+                bucket: upload_bkt,
+                key: failed_restore_key,
+                storage_class: s3_utils.STORAGE_CLASS_GLACIER,
+                xattr,
+                days: 1,
+                source_stream: buffer_utils.buffer_to_read_stream(data)
+            };
+
+            const success_params = {
+                bucket: upload_bkt,
+                key: success_restore_key,
+                storage_class: s3_utils.STORAGE_CLASS_GLACIER,
+                xattr,
+                days: 1,
+                source_stream: buffer_utils.buffer_to_read_stream(data)
+            };
+
+            const failed_file_path = glacier_ns._get_file_path(failed_params);
+            const success_file_path = glacier_ns._get_file_path(success_params);
+
+            const failure_backend = new TapeCloudGlacier();
+            failure_backend._migrate = async () => true;
+            failure_backend._process_expired = async () => { /**noop*/ };
+            failure_backend._recall = async (_file, failure_recorder, success_recorder) => {
+                // This unintentionally also replicates duplicate entries in WAL
+                await failure_recorder(failure_backend.encode_log(failed_file_path));
+
+                // This unintentionally also replicates duplicate entries in WAL
+                await success_recorder(failure_backend.encode_log(success_file_path));
 
                 return false;
             };