diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaMetadata.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaMetadata.java index 961a049d..c2dc0493 100644 --- a/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaMetadata.java +++ b/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaMetadata.java @@ -1,5 +1,5 @@ /* - * 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 @@ -7,6 +7,7 @@ */ 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; @@ -14,7 +15,9 @@ 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, 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 = "()<>@,;: innerMap; + /** + * Helper object for validating keys. + */ + private static final Predicate PREDICATE_KEY = new HttpHeaderPredicate(true); + + /** + * Helper object for validating values. + */ + private static final Predicate PREDICATE_VALUE = new HttpHeaderPredicate(false); + /** * Create a new instance backed with the specified map. + * * @param m the backing map */ public MantaMetadata(final Map m) { @@ -76,8 +95,7 @@ public MantaMetadata(final Map m) { */ public MantaMetadata() { final Map map = new CaseInsensitiveMap<>(); - final Predicate 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> 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 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 { + protected static class HttpHeaderPredicate implements Predicate { + + /** + * 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; } diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/exception/MantaInvalidMetadataException.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/exception/MantaInvalidMetadataException.java new file mode 100644 index 00000000..daaaedf1 --- /dev/null +++ b/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 Ashwin A Nair + * @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); + } +} diff --git a/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaMetadataTest.java b/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaMetadataTest.java index 0be2ec0d..bdf880e4 100644 --- a/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaMetadataTest.java +++ b/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaMetadataTest.java @@ -1,5 +1,5 @@ /* - * 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 @@ -7,13 +7,18 @@ */ 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 Elijah Zupancic + * @author Ashwin A Nair */ @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,7 +70,7 @@ public void cantAddMetadataKeyThatIllegalCharacter() { try { instance.put(key, "world"); - } catch (IllegalArgumentException e) { + } catch (final MantaInvalidMetadataException e) { caught = true; } @@ -73,6 +78,29 @@ public void cantAddMetadataKeyThatIllegalCharacter() { } } + /* 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();