Skip to content

Commit

Permalink
Added a fix for wrong status value returing InternalServer Error
Browse files Browse the repository at this point in the history
Signed-off-by: Aayush Chouhan <[email protected]>
  • Loading branch information
achouhan09 committed Jan 15, 2025
1 parent 0d2edbc commit 8160d13
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 14 deletions.
20 changes: 13 additions & 7 deletions src/endpoint/s3/ops/s3_put_bucket_lifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

const _ = require('lodash');
const s3_const = require('../s3_constants');
const s3_utils = require('../s3_utils');
const { v4: uuid } = require('uuid');
const dbg = require('../../../util/debug_module')(__filename);
const S3Error = require('../s3_errors').S3Error;
Expand Down Expand Up @@ -108,9 +109,10 @@ async function put_bucket_lifecycle(req) {
}
id_set.add(current_rule.id);

if (rule.Status?.length !== 1) {
dbg.error('Rule should have status', rule);
throw new S3Error(S3Error.InvalidArgument);
if (!rule.Status || rule.Status.length !== 1 ||
(rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_ENABLED && rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_DISABLED)) {
dbg.error(`Rule should have a status value of "${s3_const.LIFECYCLE_STATUS.STAT_ENABLED}" or "${s3_const.LIFECYCLE_STATUS.STAT_DISABLED}".`, rule);
throw new S3Error(S3Error.MalformedXML);
}
current_rule.status = rule.Status[0];

Expand Down Expand Up @@ -171,10 +173,14 @@ async function put_bucket_lifecycle(req) {
return current_rule;
});

await req.object_sdk.set_bucket_lifecycle_configuration_rules({
name: req.params.bucket,
rules: lifecycle_rules
});
try {
await req.object_sdk.set_bucket_lifecycle_configuration_rules({
name: req.params.bucket,
rules: lifecycle_rules
});
} catch (error) {
s3_utils.invalid_schema_to_aws_error(error);
}

dbg.log0('set_bucket_lifecycle', lifecycle_rules);
}
Expand Down
8 changes: 2 additions & 6 deletions src/endpoint/s3/ops/s3_put_bucket_policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
'use strict';

const S3Error = require('../s3_errors').S3Error;
const s3_utils = require('../s3_utils');

/**
* http://docs.aws.amazon.com/AmazonS3/latest/API/RESTBucketPUTpolicy.html
Expand All @@ -17,12 +18,7 @@ async function put_bucket_policy(req) {
try {
await req.object_sdk.put_bucket_policy({ name: req.params.bucket, policy });
} catch (error) {
let err = error;
if (error.rpc_code === "INVALID_SCHEMA" || error.rpc_code === "INVALID_SCHEMA_PARAMS") {
err = new S3Error(S3Error.MalformedPolicy);
err.message = "Policy was not well formed or did not validate against the published schema";
}
throw err;
s3_utils.invalid_schema_to_aws_error(error);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/endpoint/s3/s3_constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ const s3_const = exports;
///////////////

s3_const.MAX_RULE_ID_LENGTH = 255;
s3_const.LIFECYCLE_STATUS = Object.freeze({
STAT_ENABLED: 'Enabled',
STAT_DISABLED: 'Disabled'
});
3 changes: 2 additions & 1 deletion src/endpoint/s3/s3_errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ S3Error.MalformedPOSTRequest = Object.freeze({
});
S3Error.MalformedXML = Object.freeze({
code: 'MalformedXML',
message: 'This happens when the user sends malformed xml (xml that doesn\'t conform to the published xsd) for the configuration. The error message is, "The XML you provided was not well-formed or did not validate against our published schema."',
// This happens when the user sends malformed xml (xml that doesn't conform to the published xsd) for the configuration.
message: 'The XML you provided was not well-formed or did not validate against our published schema.',
http_code: 400,
});
S3Error.InvalidTag = Object.freeze({
Expand Down
19 changes: 19 additions & 0 deletions src/endpoint/s3/s3_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,24 @@ function key_marker_to_cont_tok(key_marker, objects_arr, is_truncated) {
return Buffer.from(j).toString('base64');
}

/**
* invalid_schema_to_aws_error handles specific AWS error responses related to invalid schema or parameters
* If the error's `rpc_code` is either `INVALID_SCHEMA` or `INVALID_SCHEMA_PARAMS`,
* the function throws a `MalformedPolicy` error. Otherwise, it throws the original error.
*
* @param {Object} error - The error object to handle
* @throws {Error|S3Error} - Throws an appropriate error based on the input
*/
function invalid_schema_to_aws_error(error) {
if (!error) {
throw new Error("Invalid error object: Error is undefined or null.");
}
if (["INVALID_SCHEMA", "INVALID_SCHEMA_PARAMS"].includes(error.rpc_code)) {
throw new S3Error(S3Error.MalformedPolicy);
}
throw error;
}

exports.STORAGE_CLASS_STANDARD = STORAGE_CLASS_STANDARD;
exports.STORAGE_CLASS_GLACIER = STORAGE_CLASS_GLACIER;
exports.STORAGE_CLASS_GLACIER_IR = STORAGE_CLASS_GLACIER_IR;
Expand Down Expand Up @@ -827,6 +845,7 @@ exports.get_default_object_owner = get_default_object_owner;
exports.set_response_supported_storage_classes = set_response_supported_storage_classes;
exports.cont_tok_to_key_marker = cont_tok_to_key_marker;
exports.key_marker_to_cont_tok = key_marker_to_cont_tok;
exports.invalid_schema_to_aws_error = invalid_schema_to_aws_error;
exports.parse_sse_c = parse_sse_c;
exports.OBJECT_ATTRIBUTES = OBJECT_ATTRIBUTES;
exports.OBJECT_ATTRIBUTES_UNSUPPORTED = OBJECT_ATTRIBUTES_UNSUPPORTED;
14 changes: 14 additions & 0 deletions src/test/lifecycle/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -569,3 +569,17 @@ exports.test_rule_duplicate_id = async function(Bucket, Key, s3) {
assert(error.code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
}
};

exports.test_rule_status_value = async function(Bucket, Key, s3) {
const putLifecycleParams = id_lifecycle_configuration(Bucket, Key);

// set the status value to an invalid value - other than 'Enabled' and 'Disabled'
putLifecycleParams.LifecycleConfiguration.Rules[0].Status = 'enabled';

try {
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
assert.fail('Expected MalformedXML error due to wrong status value, but received a different response');
} catch (error) {
assert(error.code === 'MalformedXML', `Expected MalformedXML error: due to invalid status value`);
}
};
1 change: 1 addition & 0 deletions src/test/system_tests/test_lifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ async function main() {
await commonTests.test_and_prefix_size(Bucket, Key, s3);
await commonTests.test_rule_id_length(Bucket, Key, s3);
await commonTests.test_rule_duplicate_id(Bucket, Key, s3);
await commonTests.test_rule_status_value(Bucket, Key, s3);

const getObjectParams = {
Bucket,
Expand Down

0 comments on commit 8160d13

Please sign in to comment.