From c91acc12f21e34416db24c126e2dbeb1b61cc313 Mon Sep 17 00:00:00 2001 From: Kanstantsin Shautsou Date: Tue, 7 Jun 2016 02:00:41 +0300 Subject: [PATCH 1/2] Implement Device parser Signed-off-by: Kanstantsin Shautsou --- .../github/dockerjava/api/model/Device.java | 69 +++++++++++++++++++ .../dockerjava/api/model/DeviceTest.java | 27 ++++++++ 2 files changed, 96 insertions(+) create mode 100644 src/test/java/com/github/dockerjava/api/model/DeviceTest.java diff --git a/src/main/java/com/github/dockerjava/api/model/Device.java b/src/main/java/com/github/dockerjava/api/model/Device.java index 9240239b1..db2bc5908 100644 --- a/src/main/java/com/github/dockerjava/api/model/Device.java +++ b/src/main/java/com/github/dockerjava/api/model/Device.java @@ -1,6 +1,8 @@ package com.github.dockerjava.api.model; import static com.google.common.base.Preconditions.checkNotNull; +import static org.apache.commons.lang.BooleanUtils.isNotTrue; +import static org.apache.commons.lang.StringUtils.isEmpty; import org.apache.commons.lang.builder.EqualsBuilder; import org.apache.commons.lang.builder.HashCodeBuilder; @@ -9,6 +11,10 @@ import com.fasterxml.jackson.annotation.JsonInclude.Include; import com.fasterxml.jackson.annotation.JsonProperty; +import javax.annotation.Nonnull; +import java.util.HashMap; +import java.util.Map; + @JsonInclude(Include.NON_NULL) public class Device { @@ -45,6 +51,69 @@ public String getPathOnHost() { return pathOnHost; } + /** + * @link https://github.com/docker/docker/blob/6b4a46f28266031ce1a1315f17fb69113a06efe1/runconfig/opts/parse_test.go#L468 + */ + @Nonnull + public static Device parse(@Nonnull String deviceStr) { + String src = ""; + String dst = ""; + String permissions = "rwm"; + final String[] arr = deviceStr.split(":"); + switch (arr.length) { + case 3: { + permissions = arr[2]; + } + case 2: { + if (validDeviceMode(arr[1])) { + permissions = arr[1]; + } else { + dst = arr[1]; + } + } + case 1: { + src = arr[0]; + break; + } + default: { + throw new IllegalArgumentException("invalid device specification: " + deviceStr); + } + } + + if (isEmpty(dst)) { + dst = src; + } + + return new Device(permissions, dst, src); + } + + /** + * ValidDeviceMode checks if the mode for device is valid or not. + * Valid mode is a composition of r (read), w (write), and m (mknod). + * + * @link https://github.com/docker/docker/blob/6b4a46f28266031ce1a1315f17fb69113a06efe1/runconfig/opts/parse.go#L796 + */ + private static boolean validDeviceMode(String deviceMode) { + Map validModes = new HashMap<>(3); + validModes.put("r", true); + validModes.put("w", true); + validModes.put("m", true); + + if (isEmpty(deviceMode)) { + return false; + } + + for (char ch : deviceMode.toCharArray()) { + final String mode = String.valueOf(ch); + if (isNotTrue(validModes.get(mode))) { + return false; // wrong mode + } + validModes.put(mode, false); + } + + return true; + } + @Override public boolean equals(Object obj) { if (obj instanceof Device) { diff --git a/src/test/java/com/github/dockerjava/api/model/DeviceTest.java b/src/test/java/com/github/dockerjava/api/model/DeviceTest.java new file mode 100644 index 000000000..25a262f1f --- /dev/null +++ b/src/test/java/com/github/dockerjava/api/model/DeviceTest.java @@ -0,0 +1,27 @@ +package com.github.dockerjava.api.model; + +import org.testng.annotations.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +/** + * @author Kanstantsin Shautsou + */ +public class DeviceTest { + @Test + public void testParse() throws Exception { + assertThat(Device.parse("/dev/sda:/dev/xvdc:r"), + equalTo(new Device("r", "/dev/xvdc", "/dev/sda"))); + + assertThat(Device.parse("/dev/snd:rw"), + equalTo(new Device("rw", "/dev/snd", "/dev/snd"))); + + assertThat(Device.parse("/dev/snd:/something"), + equalTo(new Device("rwm", "/something", "/dev/snd"))); + + assertThat(Device.parse("/dev/snd:/something:rw"), + equalTo(new Device("rw", "/something", "/dev/snd"))); + + } +} From 52f4c4e1540d8a78ba0e1f425305d7a1a4e0ed8e Mon Sep 17 00:00:00 2001 From: Kanstantsin Shautsou Date: Wed, 8 Jun 2016 01:00:30 +0300 Subject: [PATCH 2/2] Add more tests, fix. --- .../github/dockerjava/api/model/Device.java | 15 ++-- .../dockerjava/api/model/DeviceTest.java | 68 +++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/github/dockerjava/api/model/Device.java b/src/main/java/com/github/dockerjava/api/model/Device.java index db2bc5908..f2b75e3f5 100644 --- a/src/main/java/com/github/dockerjava/api/model/Device.java +++ b/src/main/java/com/github/dockerjava/api/model/Device.java @@ -14,6 +14,7 @@ import javax.annotation.Nonnull; import java.util.HashMap; import java.util.Map; +import java.util.StringTokenizer; @JsonInclude(Include.NON_NULL) public class Device { @@ -59,10 +60,16 @@ public static Device parse(@Nonnull String deviceStr) { String src = ""; String dst = ""; String permissions = "rwm"; - final String[] arr = deviceStr.split(":"); - switch (arr.length) { + final String[] arr = deviceStr.trim().split(":"); + // java String.split() returns wrong length, use tokenizer instead + switch (new StringTokenizer(deviceStr, ":").countTokens()) { case 3: { - permissions = arr[2]; + // Mismatches docker code logic. While there is no validations after parsing, checking heregit + if (validDeviceMode(arr[2])) { + permissions = arr[2]; + } else { + throw new IllegalArgumentException("Invalid device specification: " + deviceStr); + } } case 2: { if (validDeviceMode(arr[1])) { @@ -76,7 +83,7 @@ public static Device parse(@Nonnull String deviceStr) { break; } default: { - throw new IllegalArgumentException("invalid device specification: " + deviceStr); + throw new IllegalArgumentException("Invalid device specification: " + deviceStr); } } diff --git a/src/test/java/com/github/dockerjava/api/model/DeviceTest.java b/src/test/java/com/github/dockerjava/api/model/DeviceTest.java index 25a262f1f..5c48c4886 100644 --- a/src/test/java/com/github/dockerjava/api/model/DeviceTest.java +++ b/src/test/java/com/github/dockerjava/api/model/DeviceTest.java @@ -2,13 +2,61 @@ import org.testng.annotations.Test; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import static junit.framework.Assert.fail; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; /** * @author Kanstantsin Shautsou */ public class DeviceTest { + + public static List validPaths = Arrays.asList( + "/home", + "/home:/home", + "/home:/something/else", + "/with space", + "/home:/with space", + "relative:/absolute-path", + "hostPath:/containerPath:r", + "/hostPath:/containerPath:rw", + "/hostPath:/containerPath:mrw" + ); + + public static HashMap badPaths = new LinkedHashMap() {{ + put("", "bad format for path: "); + // TODO implement ValidatePath +// put("./", "./ is not an absolute path"); +// put("../", "../ is not an absolute path"); +// put("/:../", "../ is not an absolute path"); +// put("/:path", "path is not an absolute path"); +// put(":", "bad format for path: :"); +// put("/tmp:", " is not an absolute path"); +// put(":test", "bad format for path: :test"); +// put(":/test", "bad format for path: :/test"); +// put("tmp:", " is not an absolute path"); +// put(":test:", "bad format for path: :test:"); +// put("::", "bad format for path: ::"); +// put(":::", "bad format for path: :::"); +// put("/tmp:::", "bad format for path: /tmp:::"); +// put(":/tmp::", "bad format for path: :/tmp::"); +// put("path:ro", "ro is not an absolute path"); +// put("path:rr", "rr is not an absolute path"); + put("a:/b:ro", "bad mode specified: ro"); + put("a:/b:rr", "bad mode specified: rr"); + }}; + @Test public void testParse() throws Exception { assertThat(Device.parse("/dev/sda:/dev/xvdc:r"), @@ -24,4 +72,24 @@ public void testParse() throws Exception { equalTo(new Device("rw", "/something", "/dev/snd"))); } + + @Test + public void testParseBadPaths() { + for (Map.Entry entry : badPaths.entrySet()) { + final String deviceStr = entry.getKey(); + try { + Device.parse(deviceStr); + fail("Should fail because: " + entry.getValue() + " '" + deviceStr + "'"); + } catch (IllegalArgumentException ex) { + assertThat(ex.getMessage(), containsString("Invalid device specification:")); + } + } + } + + @Test + public void testParseValidPaths() { + for (String path : validPaths) { + Device.parse(path); + } + } }