Fixes #395

java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaMetadata.java

@@ -1,20 +1,23 @@
 /*
- * Copyright (c) 2015-2019, Joyent, Inc. All rights reserved.
+ * Copyright (c) 2015-2020, Joyent, Inc. All rights reserved.
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 package com.joyent.manta.client;
 
+import com.joyent.manta.exception.MantaInvalidMetadataException;
 import com.joyent.manta.util.NotThreadSafe;
 import org.apache.commons.collections4.Predicate;
 import org.apache.commons.collections4.map.CaseInsensitiveMap;
 import org.apache.commons.collections4.map.PredicatedMap;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 
 import java.io.Serializable;
+import java.nio.charset.CharsetEncoder;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Locale;
 import java.util.Map;
@@ -44,26 +47,42 @@ public class MantaMetadata implements Map<String, String>, Cloneable, Serializab
      * Prefix required for metadata keys being stored via HTTP headers on Manta.
      */
     public static final String METADATA_PREFIX = "m-";
+
     /**
      * Prefix required for encrypted metadata keys being stored in ciphertext.
      */
     public static final String ENCRYPTED_METADATA_PREFIX = "e-";
+
     /**
      * An array of characters considered to be illegal in metadata keys.
      */
     static final char[] ILLEGAL_KEY_CHARS = "()<>@,;:</[]?={}\\ \n\t\r".toCharArray();
+
     private static final long serialVersionUID = -5828336629480323042L;
+
     /**
      * The character value of the ASCII code for a space character (decimal value 32).
      */
     private static final char ASCIICODE_32_SPACE = ' ';
+
     /**
      * The backing map data structure.
      */
     private final PredicatedMap<String, String> innerMap;
 
+    /**
+     * Helper object for validating keys.
+     */
+    private static final Predicate<String> PREDICATE_KEY = new HttpHeaderPredicate(true);
+
+    /**
+     * Helper object for validating values.
+     */
+    private static final Predicate<String> PREDICATE_VALUE = new HttpHeaderPredicate(false);
+
     /**
      * Create a new instance backed with the specified map.
+     *
      * @param m the backing map
      */
     public MantaMetadata(final Map<? extends String, ? extends String> m) {
@@ -76,8 +95,7 @@ public MantaMetadata(final Map<? extends String, ? extends String> m) {
      */
     public MantaMetadata() {
         final Map<String, String> map = new CaseInsensitiveMap<>();
-        final Predicate<String> keyPredicate = new HttpHeaderNameKeyPredicate();
-        this.innerMap = PredicatedMap.predicatedMap(map, keyPredicate, null);
+        this.innerMap = PredicatedMap.predicatedMap(map, PREDICATE_KEY, PREDICATE_VALUE);
     }
 
     @SuppressWarnings("MethodDoesntCallSuperMethod")
@@ -93,11 +111,12 @@ public void removeAllEncrypted() {
         final Set<Map.Entry<String, String>> set = entrySet();
 
         set.removeIf(entry -> entry.getKey()
-           .startsWith(ENCRYPTED_METADATA_PREFIX));
+                .startsWith(ENCRYPTED_METADATA_PREFIX));
     }
 
     /**
      * Deletes user-supplied metadata associated with a Manta object.
+     *
      * @param key key to delete
      */
     public void delete(final String key) {
@@ -231,8 +250,14 @@ public void clear() {
     }
 
     @Override
-    public String put(final String key, final String value) {
-        return innerMap.put(key, value);
+    public String put(final String key, final String value) throws IllegalArgumentException {
+        try {
+            return innerMap.put(key, value);
+        } catch (IllegalArgumentException e) {
+            final String msg = String.format("[%s, %s] is in non-ASCII format",
+                    key, value);
+            throw new MantaInvalidMetadataException(msg, e);
+        }
     }
 
     @Override
@@ -253,7 +278,7 @@ public String toString() {
 
         for (Map.Entry<String, String> entry : innerMap.entrySet()) {
             builder.append(" [").append(entry.getKey()).append("] = [")
-                   .append(entry.getValue()).append("]\n");
+                    .append(entry.getValue()).append("]\n");
         }
 
         return builder.toString();
@@ -262,30 +287,54 @@ public String toString() {
     /**
      * Implements the predicate used to validate header key values.
      */
-    protected static class HttpHeaderNameKeyPredicate implements Predicate<String> {
+    protected static class HttpHeaderPredicate implements Predicate<String> {
+
+        /**
+         * ASCII-based character set encoder to determine if the input is valid.
+         */
+        private static final CharsetEncoder ASCII_ENCODER = StandardCharsets.US_ASCII.newEncoder();
+
+        /**
+         * Includes header prefix rule (m- and e-) which is only relevant for metadata header names and
+         * forbids null.
+         */
+        private final boolean keyPredicate;
+
+        /**
+         * Construct a header validator.
+         *
+         * @param keyPredicate Whether or not to include the prefix rule and forbid null.
+         */
+        protected HttpHeaderPredicate(final boolean keyPredicate) {
+            this.keyPredicate = keyPredicate;
+        }
 
         /**
          * {@inheritDoc}
          */
         @Override
         public boolean evaluate(final String object) {
-            return object != null
-                    && !object.isEmpty()
-                    && !hasIllegalChars(object)
-                    && isIso88591(object)
-                    && validPrefix(object);
+            if (keyPredicate) {
+                return object != null
+                        && !object.isEmpty()
+                        && validPrefix(object)
+                        && !hasIllegalKeyChars(object)
+                        && isAscii(object)
+                        && validPrefix(object);
+            } else {
+                // delete is put(key, null) so we must permit a null value
+                return object == null || isAscii(object);
+            }
         }
 
         /**
-         * Test a string for iso8859-1 character encoding.
+         * Test a string for US ASCII character encoding.
          *
          * @param input string value to be tested
          * @return true if the string is entirely iso8859-1, false otherwise.
          */
-        private boolean isIso88591(final String input) {
-            final byte[] bytes = input.getBytes(StandardCharsets.ISO_8859_1);
-            final String result = new String(bytes, StandardCharsets.ISO_8859_1);
-            return result.equals(input);
+        private boolean isAscii(final String input) {
+            return ASCII_ENCODER.canEncode(input);
         }
 
         /**
@@ -296,7 +345,7 @@ private boolean isIso88591(final String input) {
          */
         private boolean validPrefix(final String input) {
             return input.toLowerCase(Locale.ENGLISH).startsWith(METADATA_PREFIX)
-                   || input.toLowerCase(Locale.ENGLISH).startsWith(ENCRYPTED_METADATA_PREFIX);
+                    || input.toLowerCase(Locale.ENGLISH).startsWith(ENCRYPTED_METADATA_PREFIX);
         }
 
         /**
@@ -305,19 +354,15 @@ private boolean validPrefix(final String input) {
          * @param input string value to be tested
          * @return true if the string contains illegal characters, false otherwise.
          */
-        private boolean hasIllegalChars(final String input) {
+        private boolean hasIllegalKeyChars(final String input) {
             final char[] chars = input.toCharArray();
+            Arrays.sort(ILLEGAL_KEY_CHARS);
 
             for (final char c : chars) {
-                if (isControlCharacter(c)) {
+                final int illegalKeyCharPresent = Arrays.binarySearch(ILLEGAL_KEY_CHARS, c);
+                if (isControlCharacter(c) || illegalKeyCharPresent >= 0) {
                     return true;
                 }
-
-                for (char illegalKeyChar : ILLEGAL_KEY_CHARS) {
-                    if (c == illegalKeyChar) {
-                        return true;
-                    }
-                }
             }
 
             return false;
@@ -330,7 +375,7 @@ private boolean hasIllegalChars(final String input) {
          * @return true if the character is a control character, false otherwise.
          */
         private boolean isControlCharacter(final char c) {
-            final int intVal = (int)c;
+            final int intVal = (int) c;
             return intVal < ASCIICODE_32_SPACE;
         }
 

java-manta-client-unshaded/src/main/java/com/joyent/manta/exception/MantaInvalidMetadataException.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2020, Joyent, Inc. All rights reserved.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+package com.joyent.manta.exception;
+
+/**
+ * Exception while adding metadata.
+ *
+ * @author <a href="https://github.com/nairashwin952013">Ashwin A Nair</a>
+ * @since 3.5.0
+ */
+public class MantaInvalidMetadataException extends MantaClientException {
+
+    private static final long serialVersionUID = 3946821793051523289L;
+
+    /**
+     * @param message The error message.
+     */
+    @SuppressWarnings("unused")
+    public MantaInvalidMetadataException(final String message) {
+        super(message);
+    }
+
+    /**
+     * @param cause The cause of the exception.
+     */
+    @SuppressWarnings("unused")
+    public MantaInvalidMetadataException(final Throwable cause) {
+        super(cause);
+    }
+
+    /**
+     * @param message The error message.
+     * @param cause   The cause.
+     */
+    public MantaInvalidMetadataException(final String message, final Throwable cause) {
+        super(message, cause);
+    }
+}

java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaMetadataTest.java

@@ -1,19 +1,24 @@
 /*
- * Copyright (c) 2015-2017, Joyent, Inc. All rights reserved.
+ * Copyright (c) 2015-2020, Joyent, Inc. All rights reserved.
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 package com.joyent.manta.client;
 
+import com.joyent.manta.exception.MantaInvalidMetadataException;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import java.nio.charset.CharsetEncoder;
+import java.nio.charset.StandardCharsets;
+
 /**
  * Unit test class that verifies the correct functioning of {@link MantaMetadata}.
  *
  * @author <a href="https://github.com/dekobon">Elijah Zupancic</a>
+ * @author <a href="https://github.com/nairashwin952013">Ashwin A Nair</a>
  */
 @SuppressWarnings("ModifiedButNotUsed") // Errorprone is confused by these tests
 public class MantaMetadataTest {
@@ -24,31 +29,31 @@ public void canAddMetadataKeyValue() {
         Assert.assertEquals(instance.get("m-hello"), "world");
     }
 
-    @Test(expectedExceptions = {IllegalArgumentException.class})
+    @Test(expectedExceptions = {MantaInvalidMetadataException.class})
     public void cantAddNullMetadataKey() {
         MantaMetadata instance = new MantaMetadata();
         instance.put(null, "world");
     }
 
-    @Test(expectedExceptions = {IllegalArgumentException.class})
+    @Test(expectedExceptions = {MantaInvalidMetadataException.class})
     public void cantAddEmptyMetadataKey() {
         MantaMetadata instance = new MantaMetadata();
         instance.put("", "world");
     }
 
-    @Test(expectedExceptions = {IllegalArgumentException.class})
+    @Test(expectedExceptions = {MantaInvalidMetadataException.class})
     public void cantAddMetadataKeyThatDoesntBeginWithM() {
         MantaMetadata instance = new MantaMetadata();
         instance.put("hello", "world");
     }
 
-    @Test(expectedExceptions = {IllegalArgumentException.class})
+    @Test(expectedExceptions = {MantaInvalidMetadataException.class})
     public void cantAddMetadataKeyThatContainsSpace() {
         MantaMetadata instance = new MantaMetadata();
         instance.put("m-hello my dear", "world");
     }
 
-    @Test(expectedExceptions = {IllegalArgumentException.class})
+    @Test(expectedExceptions = {MantaInvalidMetadataException.class})
     public void cantAddMetadataKeyThatIsntISO88591() {
         MantaMetadata instance = new MantaMetadata();
         String key = "m-\u3053\u3093\u306B\u3061\u306F";
@@ -65,14 +70,37 @@ public void cantAddMetadataKeyThatIllegalCharacter() {
 
             try {
                 instance.put(key, "world");
-            } catch (IllegalArgumentException e) {
+            } catch (final MantaInvalidMetadataException e) {
                 caught = true;
             }
 
             Assert.assertTrue(caught, String.format("No exception thrown for char: %s", c));
         }
     }
 
+    /* Until MANTA-3527 is resolved we should prevent users from using non-ascii values for both keys
+    and values */
+    @Test
+    public void cantAddMetadataKeyWithNonAsciiCharacters() {
+        final CharsetEncoder asciiEncoder = StandardCharsets.US_ASCII.newEncoder();
+
+        final char[] firstNonAsciiCharacter = Character.toChars(128);
+        final String badChar = String.format("%s", firstNonAsciiCharacter[0]);
+        Assert.assertFalse(asciiEncoder.canEncode(badChar));
+
+        final MantaMetadata instance = new MantaMetadata();
+        final MantaInvalidMetadataException keyEx = Assert.expectThrows(MantaInvalidMetadataException.class, () ->
+                instance.put(String.format("m-%s", badChar), "value"));
+        final MantaInvalidMetadataException valEx = Assert.expectThrows(MantaInvalidMetadataException.class, () ->
+                instance.put("m-key", badChar));
+        final MantaInvalidMetadataException bothEx = Assert.expectThrows(MantaInvalidMetadataException.class, () ->
+                instance.put(String.format("m-%s", badChar), badChar));
+
+        Assert.assertTrue(keyEx.getMessage().contains("ASCII"));
+        Assert.assertTrue(valEx.getMessage().contains("ASCII"));
+        Assert.assertTrue(bothEx.getMessage().contains("ASCII"));
+    }
+
     @Test
     public void canMarkKeyAsDeleted() {
         MantaMetadata instance = new MantaMetadata();