Merge pull request #8338 from shirady/nsfs-nc-versioning-head-get-version-id-delete-marker

shirady · web-flow · commit 87e28c9a3df7 · 2024-09-17T08:06:16.000+03:00
NC | NSFS | Versioning | Fix Bug | Return 405 for Get/Head Specific Delete-Marker
diff --git a/src/endpoint/s3/s3_errors.js b/src/endpoint/s3/s3_errors.js
@@ -609,6 +609,7 @@ S3Error.RPC_ERRORS_TO_S3 = Object.freeze({
     NO_SUCH_TAG: S3Error.NoSuchTagSet,
     INVALID_ENCODING_TYPE: S3Error.InvalidEncodingType,
     INVALID_TARGET_BUCKET: S3Error.InvalidTargetBucketForLogging,
+    METHOD_NOT_ALLOWED: S3Error.MethodNotAllowed,
 });
 
 exports.S3Error = S3Error;
diff --git a/src/endpoint/s3/s3_rest.js b/src/endpoint/s3/s3_rest.js
@@ -397,7 +397,7 @@ function parse_op_name(req) {
     return `${method}_object`;
 }
 
-function handle_error(req, res, err) {
+function _prepare_error(req, res, err) {
     let s3err =
         ((err instanceof S3Error) && err) ||
         new S3Error(S3Error.RPC_ERRORS_TO_S3[err.rpc_code] || S3Error.InternalError);
@@ -407,19 +407,26 @@ function handle_error(req, res, err) {
         s3err.detail = err.rpc_data.detail;
     }
 
-    if (s3err.rpc_data) {
-        if (s3err.rpc_data.etag) {
+    if (err.rpc_data) {
+        if (err.rpc_data.etag) {
             if (res.headersSent) {
                 dbg.log0('Sent reply in body, bit too late for Etag header');
             } else {
-                res.setHeader('ETag', s3err.rpc_data.etag);
+                res.setHeader('ETag', err.rpc_data.etag);
             }
         }
-        if (s3err.rpc_data.last_modified) {
+        if (err.rpc_data.last_modified) {
             if (res.headersSent) {
                 dbg.log0('Sent reply in body, bit too late for Last-Modified header');
             } else {
-                res.setHeader('Last-Modified', time_utils.format_http_header_date(new Date(s3err.rpc_data.last_modified)));
+                res.setHeader('Last-Modified', time_utils.format_http_header_date(new Date(err.rpc_data.last_modified)));
+            }
+        }
+        if (err.rpc_data.delete_marker) {
+            if (res.headersSent) {
+                dbg.log0('Sent reply in body, bit too late for x-amz-delete-marker header');
+            } else {
+                res.setHeader('x-amz-delete-marker', String(err.rpc_data.delete_marker));
             }
         }
     }
@@ -432,6 +439,12 @@ function handle_error(req, res, err) {
     usage_report.s3_errors_info.total_errors += 1;
     usage_report.s3_errors_info[s3err.code] = (usage_report.s3_errors_info[s3err.code] || 0) + 1;
 
+    return s3err;
+}
+
+function handle_error(req, res, err) {
+    const s3err = _prepare_error(req, res, err);
+
     const reply = s3err.reply(req.originalUrl, req.request_id);
     dbg.error('S3 ERROR', reply,
         req.method, req.originalUrl,
@@ -450,26 +463,7 @@ function handle_error(req, res, err) {
 }
 
 async function _handle_html_response(req, res, err) {
-    let s3err =
-        ((err instanceof S3Error) && err) ||
-        new S3Error(S3Error.RPC_ERRORS_TO_S3[err.rpc_code] || S3Error.InternalError);
-
-    if (s3err.rpc_data) {
-        if (s3err.rpc_data.etag) {
-            res.setHeader('ETag', s3err.rpc_data.etag);
-        }
-        if (s3err.rpc_data.last_modified) {
-            res.setHeader('Last-Modified', time_utils.format_http_header_date(new Date(s3err.rpc_data.last_modified)));
-        }
-    }
-
-    // md_conditions used for PUT/POST/DELETE should return PreconditionFailed instead of NotModified
-    if (s3err.code === 'NotModified' && req.method !== 'HEAD' && req.method !== 'GET') {
-        s3err = new S3Error(S3Error.PreconditionFailed);
-    }
-
-    usage_report.s3_errors_info.total_errors += 1;
-    usage_report.s3_errors_info[s3err.code] = (usage_report.s3_errors_info[s3err.code] || 0) + 1;
+    const s3err = _prepare_error(req, res, err);
 
     const reply = `<html> \
         <head><title>${s3err.http_code} ${s3err.code}</title></head> \
diff --git a/src/sdk/namespace_fs.js b/src/sdk/namespace_fs.js
@@ -943,7 +943,7 @@ class NamespaceFS {
                     stat = { ...dir_content_path_stat, xattr };
                 }
             }
-            this._throw_if_delete_marker(stat);
+            this._throw_if_delete_marker(stat, params);
             return this._get_object_info(params.bucket, params.key, stat, isDir);
         } catch (err) {
             if (this._should_update_issues_report(params, file_path, err)) {
@@ -989,7 +989,7 @@ class NamespaceFS {
             );
 
             const stat = await file.stat(fs_context);
-            this._throw_if_delete_marker(stat);
+            this._throw_if_delete_marker(stat, params);
             // await this._fail_if_archived_or_sparse_file(fs_context, file_path, stat);
 
             const start = Number(params.start) || 0;
@@ -2714,11 +2714,19 @@ class NamespaceFS {
         return versioned_path;
     }
 
-    _throw_if_delete_marker(stat) {
+    _throw_if_delete_marker(stat, params) {
         if (this.versioning === versioning_status_enum.VER_ENABLED || this.versioning === versioning_status_enum.VER_SUSPENDED) {
             const xattr_delete_marker = stat.xattr[XATTR_DELETE_MARKER];
             if (xattr_delete_marker) {
-                throw error_utils.new_error_code('ENOENT', 'Entry is a delete marker');
+                const basic_err = error_utils.new_error_code('ENOENT', 'Entry is a delete marker');
+                if (params.version_id) {
+                    // If the specified version in the request is a delete marker,
+                    // the response returns a 405 Method Not Allowed error and the Last-Modified: timestamp response header.
+                    throw new RpcError('METHOD_NOT_ALLOWED',
+                        'Method not allowed, delete object id of entry delete marker',
+                        { last_modified: new Date(stat.mtime), delete_marker: true });
+                }
+                throw basic_err;
             }
         }
     }
diff --git a/src/test/unit_tests/test_bucketspace_versioning.js b/src/test/unit_tests/test_bucketspace_versioning.js
@@ -2664,6 +2664,33 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
                 assert.fail(`Failed with an error: ${err.Code}`);
             }
         });
+
+        mocha.it('head object, with version enabled, version id specified delete marker - should throw error with code 405', async function() {
+            try {
+                await s3_client.headObject({Bucket: bucket_name, Key: en_version_key, VersionId: versionID_1});
+                assert.fail('Should fail');
+            } catch (err) {
+                assert.strictEqual(err.$metadata.httpStatusCode, 405);
+                // In headObject the AWS SDK doesn't return the err.Code
+                // In AWS CLI it looks:
+                // An error occurred (405) when calling the HeadObject operation: Method Not Allowed
+                // in the docs: https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html
+                //    if the HEAD request generates an error, it returns a generic code, such as ...
+                //    405 Method Not Allowed, ... It's not possible to retrieve the exact exception of these error codes.
+            }
+        });
+
+        mocha.it('get object, with version enabled, version id specified delete marker - should throw error with code 405', async function() {
+            try {
+                await s3_client.getObject({Bucket: bucket_name, Key: en_version_key, VersionId: versionID_1});
+                assert.fail('Should fail');
+            } catch (err) {
+                assert.strictEqual(err.$metadata.httpStatusCode, 405);
+                assert.strictEqual(err.Code, 'MethodNotAllowed');
+                // In AWS CLI it looks:
+                // An error occurred (MethodNotAllowed) when calling the GetObject operation: The specified method is not allowed against this resource.
+            }
+        });
     });
 });
 
