Charles Chan
Committed by Gerrit Code Review

Fix GroupKey interpretation in REST

Input string from REST should not be converted into byte array directly.
Before: "1" -> ascii 49 -> 0x31
After: "0x01" -> 0x01
GroupKey is a byte array with arbitrary length and represented by hex string

Change-Id: If27101f0e5522212c7e434fab58b66e67e9676d7
...@@ -23,7 +23,6 @@ import java.util.Arrays; ...@@ -23,7 +23,6 @@ import java.util.Arrays;
23 * Default implementation of group key interface. 23 * Default implementation of group key interface.
24 */ 24 */
25 public class DefaultGroupKey implements GroupKey { 25 public class DefaultGroupKey implements GroupKey {
26 -
27 private final byte[] key; 26 private final byte[] key;
28 protected static final char[] HEX_ARRAY = "0123456789ABCDEF".toCharArray(); 27 protected static final char[] HEX_ARRAY = "0123456789ABCDEF".toCharArray();
29 28
...@@ -66,7 +65,6 @@ public class DefaultGroupKey implements GroupKey { ...@@ -66,7 +65,6 @@ public class DefaultGroupKey implements GroupKey {
66 hexChars[j * 2] = HEX_ARRAY[v >>> 4]; 65 hexChars[j * 2] = HEX_ARRAY[v >>> 4];
67 hexChars[j * 2 + 1] = HEX_ARRAY[v & 0x0F]; 66 hexChars[j * 2 + 1] = HEX_ARRAY[v & 0x0F];
68 } 67 }
69 - return "GroupKey:0x" + new String(hexChars); 68 + return "0x" + new String(hexChars);
70 } 69 }
71 -
72 } 70 }
...\ No newline at end of file ...\ No newline at end of file
......
...@@ -18,6 +18,7 @@ package org.onosproject.codec.impl; ...@@ -18,6 +18,7 @@ package org.onosproject.codec.impl;
18 import com.fasterxml.jackson.databind.JsonNode; 18 import com.fasterxml.jackson.databind.JsonNode;
19 import com.fasterxml.jackson.databind.node.ArrayNode; 19 import com.fasterxml.jackson.databind.node.ArrayNode;
20 import com.fasterxml.jackson.databind.node.ObjectNode; 20 import com.fasterxml.jackson.databind.node.ObjectNode;
21 +import org.onlab.util.HexString;
21 import org.onosproject.codec.CodecContext; 22 import org.onosproject.codec.CodecContext;
22 import org.onosproject.codec.JsonCodec; 23 import org.onosproject.codec.JsonCodec;
23 import org.onosproject.core.ApplicationId; 24 import org.onosproject.core.ApplicationId;
...@@ -118,7 +119,11 @@ public final class GroupCodec extends JsonCodec<Group> { ...@@ -118,7 +119,11 @@ public final class GroupCodec extends JsonCodec<Group> {
118 // parse group key (appCookie) 119 // parse group key (appCookie)
119 String groupKeyStr = nullIsIllegal(json.get(APP_COOKIE), 120 String groupKeyStr = nullIsIllegal(json.get(APP_COOKIE),
120 APP_COOKIE + MISSING_MEMBER_MESSAGE).asText(); 121 APP_COOKIE + MISSING_MEMBER_MESSAGE).asText();
121 - GroupKey groupKey = new DefaultGroupKey(groupKeyStr.getBytes()); 122 + if (!groupKeyStr.startsWith("0x")) {
123 + throw new IllegalArgumentException("APP_COOKIE must be a hex string starts with 0x");
124 + }
125 + GroupKey groupKey = new DefaultGroupKey(HexString.fromHexString(
126 + groupKeyStr.split("0x")[1], ""));
122 127
123 // parse device id 128 // parse device id
124 DeviceId deviceId = DeviceId.deviceId(nullIsIllegal(json.get(DEVICE_ID), 129 DeviceId deviceId = DeviceId.deviceId(nullIsIllegal(json.get(DEVICE_ID),
......
...@@ -44,6 +44,7 @@ import static org.easymock.EasyMock.replay; ...@@ -44,6 +44,7 @@ import static org.easymock.EasyMock.replay;
44 import static org.hamcrest.MatcherAssert.assertThat; 44 import static org.hamcrest.MatcherAssert.assertThat;
45 import static org.hamcrest.Matchers.is; 45 import static org.hamcrest.Matchers.is;
46 import static org.hamcrest.Matchers.notNullValue; 46 import static org.hamcrest.Matchers.notNullValue;
47 +import static org.hamcrest.Matchers.equalTo;
47 import static org.onosproject.codec.impl.GroupJsonMatcher.matchesGroup; 48 import static org.onosproject.codec.impl.GroupJsonMatcher.matchesGroup;
48 import static org.onosproject.net.NetTestTools.APP_ID; 49 import static org.onosproject.net.NetTestTools.APP_ID;
49 50
...@@ -109,6 +110,11 @@ public class GroupCodecTest { ...@@ -109,6 +110,11 @@ public class GroupCodecTest {
109 assertThat(((Instructions.OutputInstruction) instruction1).port(), is(PortNumber.portNumber(2))); 110 assertThat(((Instructions.OutputInstruction) instruction1).port(), is(PortNumber.portNumber(2)));
110 } 111 }
111 112
113 + @Test(expected = IllegalArgumentException.class)
114 + public void invalidGroupTest() throws IOException {
115 + Group group = getGroup("invalid-group.json");
116 + }
117 +
112 /** 118 /**
113 * Checks that the data shared by all the resource is correct for a given group. 119 * Checks that the data shared by all the resource is correct for a given group.
114 * 120 *
...@@ -118,7 +124,8 @@ public class GroupCodecTest { ...@@ -118,7 +124,8 @@ public class GroupCodecTest {
118 assertThat(group.appId(), is(APP_ID)); 124 assertThat(group.appId(), is(APP_ID));
119 assertThat(group.deviceId().toString(), is("of:0000000000000001")); 125 assertThat(group.deviceId().toString(), is("of:0000000000000001"));
120 assertThat(group.type().toString(), is("ALL")); 126 assertThat(group.type().toString(), is("ALL"));
121 - assertThat(group.appCookie().key(), is("1".getBytes())); 127 + assertThat(group.appCookie().key(),
128 + equalTo(new byte[]{(byte) 0x12, (byte) 0x34, (byte) 0xAB, (byte) 0xCD}));
122 assertThat(group.id().id(), is(1)); 129 assertThat(group.id().id(), is(1));
123 } 130 }
124 131
......
1 +{
2 + "type": "ALL",
3 + "deviceId": "of:0000000000000001",
4 + "appCookie": "1234abCD",
5 + "groupId": "1",
6 + "buckets": [
7 + {
8 + "treatment": {
9 + "instructions": [
10 + {
11 + "type": "OUTPUT",
12 + "port": 2
13 + }
14 + ]
15 + }
16 + }
17 + ]
18 +}
...\ No newline at end of file ...\ No newline at end of file
1 { 1 {
2 "type": "ALL", 2 "type": "ALL",
3 "deviceId": "of:0000000000000001", 3 "deviceId": "of:0000000000000001",
4 - "appCookie": "1", 4 + "appCookie": "0x1234abCD",
5 "groupId": "1", 5 "groupId": "1",
6 "buckets": [ 6 "buckets": [
7 { 7 {
......
...@@ -67,7 +67,7 @@ ...@@ -67,7 +67,7 @@
67 "appCookie": { 67 "appCookie": {
68 "type": "string", 68 "type": "string",
69 "description": "application cookie", 69 "description": "application cookie",
70 - "example": "1" 70 + "example": "0x1234abcd"
71 }, 71 },
72 "buckets": { 72 "buckets": {
73 "type": "array", 73 "type": "array",
......
...@@ -67,11 +67,6 @@ ...@@ -67,11 +67,6 @@
67 "description": "types of the group", 67 "description": "types of the group",
68 "example": "ALL" 68 "example": "ALL"
69 }, 69 },
70 - "deviceId": {
71 - "type": "string",
72 - "description": "device identifier",
73 - "example": "of:0000000000000003"
74 - },
75 "appId": { 70 "appId": {
76 "type": "string", 71 "type": "string",
77 "description": "application identifier", 72 "description": "application identifier",
...@@ -80,7 +75,7 @@ ...@@ -80,7 +75,7 @@
80 "appCookie": { 75 "appCookie": {
81 "type": "string", 76 "type": "string",
82 "description": "application cookie", 77 "description": "application cookie",
83 - "example": "1" 78 + "example": "0x1234abcd"
84 }, 79 },
85 "buckets": { 80 "buckets": {
86 "type": "array", 81 "type": "array",
......
...@@ -19,7 +19,8 @@ ...@@ -19,7 +19,8 @@
19 }, 19 },
20 "appCookie": { 20 "appCookie": {
21 "type": "string", 21 "type": "string",
22 - "example": "1" 22 + "description": "application cookie. Arbitrary length byte array represented in hex string",
23 + "example": "0x1234abcd"
23 }, 24 },
24 "groupId": { 25 "groupId": {
25 "type": "string", 26 "type": "string",
......
1 { 1 {
2 "type": "ALL", 2 "type": "ALL",
3 "deviceId": "of:0000000000000001", 3 "deviceId": "of:0000000000000001",
4 - "appCookie": "1", 4 + "appCookie": "0x1",
5 "groupId": "1", 5 "groupId": "1",
6 "buckets": [ 6 "buckets": [
7 { 7 {
......