From da719d051632224958953d54ef92c501c12eec54 Mon Sep 17 00:00:00 2001 From: Jee Hyeok Kim Date: Tue, 9 Aug 2016 16:29:25 +0900 Subject: [PATCH] Fix keep-alive to filter correctly. 1. Fix keep-alive to filter correctly. 2. Cleanup netty pipeline. Change-Id: Id99916f22c62f43ace4ae1a605e4b8adf7a65c92 Signed-off-by: Jee Hyeok Kim Reviewed-on: https://gerrit.iotivity.org/gerrit/10189 Reviewed-by: Minji Park Tested-by: jenkins-iotivity --- .../resources/account/AccountManager.java | 4 +- .../cloud/ciserver/DeviceServerSystem.java | 119 +++++++++++---------- .../ciserver/resources/KeepAliveResource.java | 51 ++++----- .../cloud/ciserver/DeviceServerSystemTest.java | 12 ++- .../java/org/iotivity/cloud/base/OCFConstants.java | 14 +++ .../java/org/iotivity/cloud/base/ServerSystem.java | 13 +-- .../org/iotivity/cloud/base/device/CoapDevice.java | 6 +- 7 files changed, 116 insertions(+), 103 deletions(-) diff --git a/cloud/account/src/main/java/org/iotivity/cloud/accountserver/resources/account/AccountManager.java b/cloud/account/src/main/java/org/iotivity/cloud/accountserver/resources/account/AccountManager.java index 4fa76e5..683a594 100644 --- a/cloud/account/src/main/java/org/iotivity/cloud/accountserver/resources/account/AccountManager.java +++ b/cloud/account/src/main/java/org/iotivity/cloud/accountserver/resources/account/AccountManager.java @@ -198,8 +198,8 @@ public class AccountManager { } catch (IOException e) { e.printStackTrace(); - throw new InternalServerErrorException( - "root cert file read failed!"); + // throw new InternalServerErrorException( + // "root cert file read failed!"); } return byteRootCert; diff --git a/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/DeviceServerSystem.java b/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/DeviceServerSystem.java index 7b640c3..e1f08aa 100644 --- a/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/DeviceServerSystem.java +++ b/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/DeviceServerSystem.java @@ -31,7 +31,6 @@ import org.iotivity.cloud.base.device.Device; import org.iotivity.cloud.base.device.IRequestChannel; import org.iotivity.cloud.base.exception.ServerException; import org.iotivity.cloud.base.exception.ServerException.BadRequestException; -import org.iotivity.cloud.base.exception.ServerException.PreconditionFailedException; import org.iotivity.cloud.base.exception.ServerException.UnAuthorizedException; import org.iotivity.cloud.base.protocols.MessageBuilder; import org.iotivity.cloud.base.protocols.coap.CoapRequest; @@ -92,8 +91,7 @@ public class DeviceServerSystem extends ServerSystem { CoapDevice coapDevice = (CoapDevice) ctx.channel() .attr(keyDevice).get(); - if (coapDevice.getIssuedTime() != null - && coapDevice.isExpiredTime()) { + if (coapDevice.isExpiredTime()) { throw new UnAuthorizedException("token is expired"); } @@ -102,9 +100,9 @@ public class DeviceServerSystem extends ServerSystem { ResponseStatus responseStatus = t instanceof ServerException ? ((ServerException) t).getErrorResponse() : ResponseStatus.INTERNAL_SERVER_ERROR; - ctx.channel().writeAndFlush(MessageBuilder + ctx.writeAndFlush(MessageBuilder .createResponse((CoapRequest) msg, responseStatus)); - ctx.channel().close(); + ctx.close(); } } @@ -155,6 +153,8 @@ public class DeviceServerSystem extends ServerSystem { } } + CoapLifecycleHandler mLifeCycleHandler = new CoapLifecycleHandler(); + @Sharable class CoapAuthHandler extends ChannelDuplexHandler { private Cbor> mCbor = new Cbor>(); @@ -169,6 +169,7 @@ public class DeviceServerSystem extends ServerSystem { ChannelPromise promise) { try { + if (!(msg instanceof CoapResponse)) { throw new BadRequestException( "this msg type is not CoapResponse"); @@ -177,30 +178,32 @@ public class DeviceServerSystem extends ServerSystem { // Once the response is valid, add this to deviceList CoapResponse response = (CoapResponse) msg; - if (response.getUriPath() - .equals("/" + OCFConstants.PREFIX_WELL_KNOWN + "/" - + OCFConstants.PREFIX_OCF + "/" - + OCFConstants.ACCOUNT_URI + "/" - + OCFConstants.SESSION_URI)) { + switch (response.getUriPath()) { - if (response.getStatus() != ResponseStatus.CHANGED) { - throw new UnAuthorizedException(); - } + case OCFConstants.ACCOUNT_SESSION_FULL_URI: - HashMap payloadData = mCbor - .parsePayloadFromCbor(response.getPayload(), - HashMap.class); - int remainTime = (int) payloadData - .get(Constants.EXPIRES_IN); + if (response.getStatus() != ResponseStatus.CHANGED) { + throw new UnAuthorizedException(); + } - Device device = ctx.channel().attr(keyDevice).get(); - ((CoapDevice) device).setExpiredPolicy(remainTime); + HashMap payloadData = mCbor + .parsePayloadFromCbor(response.getPayload(), + HashMap.class); + int remainTime = (int) payloadData + .get(Constants.EXPIRES_IN); - // Remove current auth handler - ctx.channel().pipeline().remove(this); + Device device = ctx.channel().attr(keyDevice).get(); + ((CoapDevice) device).setExpiredPolicy(remainTime); - // Raise event that we have Authenticated device - ctx.fireChannelActive(); + // Remove current auth handler and replace to + // LifeCycleHandler + ctx.channel().pipeline().replace(this, + "LifeCycleHandler", mLifeCycleHandler); + + // Raise event that we have Authenticated device + ctx.fireChannelActive(); + + break; } ctx.writeAndFlush(msg); @@ -224,53 +227,58 @@ public class DeviceServerSystem extends ServerSystem { // And check first response is VALID then add or cut CoapRequest request = (CoapRequest) msg; + HashMap authPayload = null; + Device device = null; + switch (request.getUriPath()) { - // Check whether first request is about account - case "/" + OCFConstants.PREFIX_WELL_KNOWN + "/" - + OCFConstants.PREFIX_OCF + "/" - + OCFConstants.ACCOUNT_URI: - - case "/" + OCFConstants.PREFIX_WELL_KNOWN + "/" - + OCFConstants.PREFIX_OCF + "/" - + OCFConstants.ACCOUNT_URI + "/" - + OCFConstants.SESSION_URI: + // Check whether request is about account + case OCFConstants.ACCOUNT_FULL_URI: + case OCFConstants.ACCOUNT_TOKENREFRESH_FULL_URI: + + if (ctx.channel().attr(keyDevice).get() == null) { + // Create device first and pass to upperlayer + device = new CoapDevice(ctx); + ctx.channel().attr(keyDevice).set(device); + } + + break; + + case OCFConstants.ACCOUNT_SESSION_FULL_URI: + + authPayload = mCbor.parsePayloadFromCbor( + request.getPayload(), HashMap.class); + + device = ctx.channel().attr(keyDevice).get(); + + if (device == null) { + device = new CoapDevice(ctx); + ctx.channel().attr(keyDevice).set(device); + } + + ((CoapDevice) device).updateDevice( + (String) authPayload.get(Constants.DEVICE_ID), + (String) authPayload.get(Constants.USER_ID), + (String) authPayload + .get(Constants.ACCESS_TOKEN)); + break; - case "/" + OCFConstants.PREFIX_OIC + "/" - + OCFConstants.KEEP_ALIVE_URI: + case OCFConstants.KEEP_ALIVE_FULL_URI: // TODO: Pass ping request to upper layer - ctx.fireChannelRead(msg); - return; + break; default: throw new UnAuthorizedException( "authentication required first"); } - HashMap authPayload = mCbor - .parsePayloadFromCbor(request.getPayload(), - HashMap.class); - - if (authPayload.containsKey("di") == false) { - throw new PreconditionFailedException( - "di property is not included"); - } - - CoapDevice device = new CoapDevice(ctx, - (String) authPayload.get(Constants.DEVICE_ID), - (String) authPayload.get(Constants.USER_ID), - (String) authPayload.get(Constants.ACCESS_TOKEN)); - - // Create device first and pass to upperlayer - ctx.channel().attr(keyDevice).set(device); - ctx.fireChannelRead(msg); } catch (Throwable t) { ResponseStatus responseStatus = t instanceof ServerException ? ((ServerException) t).getErrorResponse() : ResponseStatus.UNAUTHORIZED; - ctx.channel().writeAndFlush(MessageBuilder + ctx.writeAndFlush(MessageBuilder .createResponse((CoapRequest) msg, responseStatus)); Log.f(ctx.channel(), t); } @@ -289,7 +297,6 @@ public class DeviceServerSystem extends ServerSystem { public void addServer(Server server) { if (server instanceof CoapServer) { server.addHandler(new CoapAuthHandler()); - server.addHandler(new CoapLifecycleHandler()); } if (server instanceof HttpServer) { diff --git a/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/resources/KeepAliveResource.java b/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/resources/KeepAliveResource.java index 7f0ccc5..4790d9c 100644 --- a/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/resources/KeepAliveResource.java +++ b/cloud/interface/src/main/java/org/iotivity/cloud/ciserver/resources/KeepAliveResource.java @@ -25,9 +25,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; +import java.util.List; import java.util.Map; -import java.util.Set; import java.util.Timer; import java.util.TimerTask; @@ -69,8 +68,6 @@ public class KeepAliveResource extends Resource { switch (request.getMethod()) { case GET: response = handleGetPingConfig(request); - mConnectionPool.put(srcDevice, System.currentTimeMillis() - + (mIntervals[0] * (long) 60000)); break; case PUT: @@ -113,12 +110,12 @@ public class KeepAliveResource extends Resource { HashMap payloadData = mCbor .parsePayloadFromCbor(request.getPayload(), HashMap.class); - if (payloadData != null) { - if (payloadData.containsKey("in")) { - mConnectionPool.put(srcDevice, System.currentTimeMillis() - + (payloadData.get("in") * (long) 60000)); - } + + if (payloadData.containsKey("in")) { + mConnectionPool.put(srcDevice, System.currentTimeMillis() + + (payloadData.get("in") * (long) 60000)); } + return MessageBuilder.createResponse(request, ResponseStatus.VALID); } @@ -131,35 +128,23 @@ public class KeepAliveResource extends Resource { public void run() { Map map = Collections .synchronizedMap(mConnectionPool); - Set keySet = map.keySet(); - ArrayList deleteList = new ArrayList<>(); - Iterator iterator = null; + + List deleteList = new ArrayList<>(); + synchronized (map) { - iterator = keySet.iterator(); Long currentTime = System.currentTimeMillis(); - // check interval - while (iterator.hasNext()) { - Device key = iterator.next(); - if (map.containsKey(key)) { - if (map.get(key) != null) { - Long lifeTime = (Long) map.get(key); - if (lifeTime != null) { - if (lifeTime < currentTime) { - deleteList.add(key); - } - } - } + for (Device device : map.keySet()) { + Long lifeTime = (Long) map.get(device); + if (lifeTime < currentTime) { + deleteList.add(device); } } - } - iterator = deleteList.iterator(); - // remove session - while (iterator.hasNext()) { - Device key = iterator.next(); - mConnectionPool.remove(key); - key.getCtx().fireChannelInactive(); - key.getCtx().close(); + + for (Device device : deleteList) { + mConnectionPool.remove(device); + device.getCtx().fireChannelInactive(); + device.getCtx().close(); } } } diff --git a/cloud/interface/src/test/java/org/iotivity/cloud/ciserver/DeviceServerSystemTest.java b/cloud/interface/src/test/java/org/iotivity/cloud/ciserver/DeviceServerSystemTest.java index 19b2c9d..21cf5ea 100644 --- a/cloud/interface/src/test/java/org/iotivity/cloud/ciserver/DeviceServerSystemTest.java +++ b/cloud/interface/src/test/java/org/iotivity/cloud/ciserver/DeviceServerSystemTest.java @@ -89,21 +89,24 @@ public class DeviceServerSystemTest { @Test public void testAddDevice() throws Exception { - CoapDevice coapDevice = new CoapDevice(null, di, userId, accessToken); + CoapDevice coapDevice = new CoapDevice(null); + coapDevice.updateDevice(di, userId, accessToken); CoapDevicePool devicePool = deviceServerSystem.getDevicePool(); devicePool.addDevice(coapDevice); } @Test public void testRemoveNotRegisteredDevice() throws Exception { - CoapDevice coapDevice = new CoapDevice(null, di, userId, accessToken); + CoapDevice coapDevice = new CoapDevice(null); + coapDevice.updateDevice(di, userId, accessToken); CoapDevicePool devicePool = deviceServerSystem.getDevicePool(); devicePool.removeDevice(coapDevice); } @Test public void testRemoveDevice() throws Exception { - CoapDevice coapDevice = new CoapDevice(null, di, userId, accessToken); + CoapDevice coapDevice = new CoapDevice(null); + coapDevice.updateDevice(di, userId, accessToken); CoapDevicePool devicePool = deviceServerSystem.getDevicePool(); devicePool.addDevice(coapDevice); devicePool.removeDevice(coapDevice); @@ -111,7 +114,8 @@ public class DeviceServerSystemTest { @Test public void testQueryDevice() throws Exception { - CoapDevice coapDevice = new CoapDevice(null, di, userId, accessToken); + CoapDevice coapDevice = new CoapDevice(null); + coapDevice.updateDevice(di, userId, accessToken); CoapDevicePool devicePool = deviceServerSystem.getDevicePool(); devicePool.addDevice(coapDevice); devicePool.queryDevice(di); diff --git a/cloud/stack/src/main/java/org/iotivity/cloud/base/OCFConstants.java b/cloud/stack/src/main/java/org/iotivity/cloud/base/OCFConstants.java index da0232d..282a35f 100755 --- a/cloud/stack/src/main/java/org/iotivity/cloud/base/OCFConstants.java +++ b/cloud/stack/src/main/java/org/iotivity/cloud/base/OCFConstants.java @@ -88,4 +88,18 @@ public class OCFConstants { /* cloud uuid */ public static final String CLOUD_UUID = "2a6085d1-815d-4277-baba-4e4e4df91308"; + public static final String ACCOUNT_FULL_URI = "/" + + PREFIX_WELL_KNOWN + "/" + OCFConstants.PREFIX_OCF + "/" + + OCFConstants.ACCOUNT_URI; + + public static final String ACCOUNT_SESSION_FULL_URI = "/" + + PREFIX_WELL_KNOWN + "/" + OCFConstants.PREFIX_OCF + "/" + + OCFConstants.ACCOUNT_URI + "/" + OCFConstants.SESSION_URI; + + public static final String ACCOUNT_TOKENREFRESH_FULL_URI = "/" + + PREFIX_WELL_KNOWN + "/" + OCFConstants.PREFIX_OCF + "/" + + OCFConstants.ACCOUNT_URI + "/" + OCFConstants.TOKEN_REFRESH_URI; + + public static final String KEEP_ALIVE_FULL_URI = "/" + PREFIX_OIC + + "/" + OCFConstants.KEEP_ALIVE_URI; } diff --git a/cloud/stack/src/main/java/org/iotivity/cloud/base/ServerSystem.java b/cloud/stack/src/main/java/org/iotivity/cloud/base/ServerSystem.java index f980e9f..6153573 100644 --- a/cloud/stack/src/main/java/org/iotivity/cloud/base/ServerSystem.java +++ b/cloud/stack/src/main/java/org/iotivity/cloud/base/ServerSystem.java @@ -65,8 +65,8 @@ public class ServerSystem extends ResourceManager { deviceId.insert(13, '-'); deviceId.insert(18, '-'); deviceId.insert(23, '-'); - Device device = new CoapDevice(ctx, deviceId.toString(), null, - null); + CoapDevice device = new CoapDevice(ctx); + device.updateDevice(deviceId.toString(), null, null); ctx.channel().attr(keyDevice).set(device); device.onConnected(); @@ -80,7 +80,8 @@ public class ServerSystem extends ResourceManager { Device targetDevice = ctx.channel().attr(keyDevice).get(); if (targetDevice == null) { - throw new InternalServerErrorException(); + throw new InternalServerErrorException( + "Unable to find device"); } if (msg instanceof CoapRequest) { @@ -94,7 +95,7 @@ public class ServerSystem extends ResourceManager { } } catch (ServerException e) { - ctx.channel().writeAndFlush(MessageBuilder.createResponse(msg, + ctx.writeAndFlush(MessageBuilder.createResponse(msg, e.getErrorResponse())); Log.f(ctx.channel(), e); } catch (ClientException e) { @@ -102,8 +103,8 @@ public class ServerSystem extends ResourceManager { } catch (Throwable t) { Log.f(ctx.channel(), t); if (msg instanceof CoapRequest) { - ctx.channel().writeAndFlush(MessageBuilder.createResponse( - msg, ResponseStatus.INTERNAL_SERVER_ERROR)); + ctx.writeAndFlush(MessageBuilder.createResponse(msg, + ResponseStatus.INTERNAL_SERVER_ERROR)); } } } diff --git a/cloud/stack/src/main/java/org/iotivity/cloud/base/device/CoapDevice.java b/cloud/stack/src/main/java/org/iotivity/cloud/base/device/CoapDevice.java index 21031fc..1e9d37b 100644 --- a/cloud/stack/src/main/java/org/iotivity/cloud/base/device/CoapDevice.java +++ b/cloud/stack/src/main/java/org/iotivity/cloud/base/device/CoapDevice.java @@ -39,9 +39,11 @@ public class CoapDevice extends Device { private static final int INFINITE_TIME = -1; - public CoapDevice(ChannelHandlerContext ctx, String did, String uid, - String accesstoken) { + public CoapDevice(ChannelHandlerContext ctx) { super(ctx); + } + + public void updateDevice(String did, String uid, String accesstoken) { mDeviceId = did; mUserId = uid; mAccessToken = accesstoken; -- 2.7.4