NC | Revert url encode/decode of xattr values

src/endpoint/s3/s3_utils.js

@@ -64,7 +64,7 @@ function get_request_xattr(req) {
         if (!hdr.startsWith(X_AMZ_META)) return;
         const key = hdr.slice(X_AMZ_META.length);
         if (!key) return;
-        xattr[key] = decodeURIComponent(val);
+        xattr[key] = val;
     });
     return xattr;
 }
@@ -82,9 +82,7 @@ function set_response_xattr(res, xattr) {
     }
     let returned_keys = 0;
     for (const key of keys) {
-        // when xattr is set directly on the object (NSFS for example) and it's already encoded
-        // we should not encode it again 
-        const val = encode_uri_unless_already_encoded(xattr[key]);
+        const val = xattr[key];
 
         const md_header_size =
             X_AMZ_META.length +
@@ -97,7 +95,11 @@ function set_response_xattr(res, xattr) {
         }
         returned_keys += 1;
         size_for_md_left -= md_header_size;
-        res.setHeader(X_AMZ_META + key, val);
+        try {
+            res.setHeader(X_AMZ_META + key, val);
+        } catch (err) {
+            dbg.warn(`s3_utils.set_response_xattr set_header failed, skipping... res.req.url=${res.req?.url} xattr key=${key} xattr value=${val}`);
+        }
     }
 }
 
@@ -629,15 +631,6 @@ function parse_decimal_int(str) {
     return parsed;
 }
 
-/**
- * encode_uri_unless_already_encoded encodes a string uri if it's not already encoded
- * @param {string} uri
- * @returns {string}
- */
-function encode_uri_unless_already_encoded(uri = '') {
-    return is_uri_already_encoded(uri) ? uri : encodeURIComponent(uri);
-}
-
 /**
  * parse_version_id throws an error if version_id is an empty string, and returns it otherwise
  * @param {string|undefined} version_id
@@ -650,15 +643,6 @@ function parse_version_id(version_id, empty_err = S3Error.InvalidArgumentEmptyVe
     return version_id;
 }
 
-/**
- * is_uri_already_encoded returns true if string uri is URIEncoded
- * @param {string} uri
- * @returns {boolean}
- */
-function is_uri_already_encoded(uri = '') {
-    return uri !== decodeURIComponent(uri);
-}
-
 /**
  * 
  * @param {*} req 

src/test/unit_tests/test_bucketspace.js

@@ -1,6 +1,6 @@
 /* Copyright (C) 2020 NooBaa */
 /*eslint max-lines: ["error", 2200]*/
-/*eslint max-lines-per-function: ["error", 900]*/
+/*eslint max-lines-per-function: ["error", 1300]*/
 /*eslint max-statements: ["error", 80, { "ignoreTopLevelFunctions": true }]*/
 'use strict';
 
@@ -562,25 +562,7 @@ mocha.describe('bucket operations - namespace_fs', function() {
         await s3_client.deleteObject({ Bucket: bucket, Key: key });
     });
 
-    mocha.it('putObject/head object - xattr invalid URI', async function() {
-        const { access_key, secret_key } = account_correct_uid.access_keys[0];
-        const s3_client = generate_s3_client(access_key.unwrap(), secret_key.unwrap(), CORETEST_ENDPOINT);
-        const bucket = bucket_name + '-s3';
-        const key = 'obj_dot_xattr.txt';
-        const xattr = { 'key1.2.3': encoded_xattr };
-        const policy = generate_s3_policy('*', bucket, ['s3:*']);
-        await rpc_client.bucket.put_bucket_policy({ name: bucket, policy: policy.policy });
-
-        await s3_client.putObject({ Bucket: bucket, Key: key, Body: generic_obj_body, Metadata: xattr });
-        const head_res = await s3_client.headObject({ Bucket: bucket, Key: key });
-
-        assert.deepStrictEqual(head_res.Metadata, xattr);
-        await s3_client.deleteObject({ Bucket: bucket, Key: key });
-    });
-
     mocha.it('PUT file directly to FS/head object - xattr decoded invalid URI', async function() {
-        //const s3_creds_aws_sdk_v2 = get_aws_sdk_v2_base_params(account_correct_uid, s3_endpoint_address);
-        //const s3 = new AWS.S3(s3_creds_aws_sdk_v2);
         const { access_key, secret_key } = account_correct_uid.access_keys[0];
         const s3_client = generate_s3_client(access_key.unwrap(), secret_key.unwrap(), CORETEST_ENDPOINT);
         const key = 'fs_obj_dot_xattr.txt';
@@ -589,14 +571,16 @@ mocha.describe('bucket operations - namespace_fs', function() {
 
         const tmpfile = await open(DEFAULT_FS_CONFIG, PATH, 'w');
         const fs_xattr = { 'user.key1.2.3': decoded_xattr };
-        const s3_xattr = { 'key1.2.3': encoded_xattr };
+        const s3_xattr = {}; // invalid xattr won't return on s3 head object
         await tmpfile.replacexattr(DEFAULT_FS_CONFIG, fs_xattr);
         const xattr_res = (await tmpfile.stat(DEFAULT_FS_CONFIG)).xattr;
         await tmpfile.close(DEFAULT_FS_CONFIG);
 
         const head_res = await s3_client.headObject({ Bucket: bucket, Key: key });
         assert.deepStrictEqual(head_res.Metadata, s3_xattr);
         assert.deepStrictEqual(fs_xattr, xattr_res);
+        const get_res = await s3_client.getObject({ Bucket: bucket, Key: key });
+        assert.deepStrictEqual(get_res.Metadata, s3_xattr);
         await s3_client.deleteObject({ Bucket: bucket, Key: key });
     });
 
@@ -618,6 +602,8 @@ mocha.describe('bucket operations - namespace_fs', function() {
         const head_res = await s3_client.headObject({ Bucket: bucket, Key: key });
         assert.deepStrictEqual(head_res.Metadata, s3_xattr);
         assert.deepStrictEqual(fs_xattr, xattr_res);
+        const get_res = await s3_client.getObject({ Bucket: bucket, Key: key });
+        assert.deepStrictEqual(get_res.Metadata, s3_xattr);
         await s3_client.deleteObject({ Bucket: bucket, Key: key });
     });
 
@@ -1764,7 +1750,6 @@ mocha.describe('s3 whitelist flow', async function() {
     });
 });
 
-/*eslint max-lines-per-function: ["error", 1300]*/
 mocha.describe('Namespace s3_bucket_policy', function() {
     this.timeout(600000); // eslint-disable-line no-invalid-this
     const anon_access_policy = {