From 7c5dfaee20588ed24164a4a695535cf995206934 Mon Sep 17 00:00:00 2001 From: zzz Date: Wed, 17 Jun 2015 23:44:12 +0000 Subject: [PATCH] I2CP: More fixes after prop, w.r.t. restore after close-on-idle - When socket is closed, set sessionID and LS to null, close subsession and set its sessionID and LS to null - Checks on client side for null session ID - Check for null session in Destroy Session message - Don't kill I2CP connection due to a bad session ID in a SendMessage, just drop the message and send a MessageStatusMessage - Log tweaks --- .../net/i2p/client/I2CPMessageProducer.java | 29 ++++++++++-- .../src/net/i2p/client/I2PSessionImpl.java | 22 +++++++-- history.txt | 4 ++ .../client/ClientMessageEventListener.java | 47 +++++++++++++++---- 4 files changed, 85 insertions(+), 17 deletions(-) diff --git a/core/java/src/net/i2p/client/I2CPMessageProducer.java b/core/java/src/net/i2p/client/I2CPMessageProducer.java index c625e7841..bbabc3a11 100644 --- a/core/java/src/net/i2p/client/I2CPMessageProducer.java +++ b/core/java/src/net/i2p/client/I2CPMessageProducer.java @@ -34,6 +34,7 @@ import net.i2p.data.i2cp.ReportAbuseMessage; import net.i2p.data.i2cp.SendMessageMessage; import net.i2p.data.i2cp.SendMessageExpiresMessage; import net.i2p.data.i2cp.SessionConfig; +import net.i2p.data.i2cp.SessionId; import net.i2p.util.Log; /** @@ -154,7 +155,12 @@ class I2CPMessageProducer { } else msg = new SendMessageMessage(); msg.setDestination(dest); - msg.setSessionId(session.getSessionId()); + SessionId sid = session.getSessionId(); + if (sid == null) { + _log.error(session.toString() + " send message w/o session", new Exception()); + return; + } + msg.setSessionId(sid); msg.setNonce(nonce); Payload data = createPayload(dest, payload, null, null, null, null); msg.setPayload(data); @@ -176,7 +182,12 @@ class I2CPMessageProducer { return; SendMessageMessage msg = new SendMessageExpiresMessage(options); msg.setDestination(dest); - msg.setSessionId(session.getSessionId()); + SessionId sid = session.getSessionId(); + if (sid == null) { + _log.error(session.toString() + " send message w/o session", new Exception()); + return; + } + msg.setSessionId(sid); msg.setNonce(nonce); Payload data = createPayload(dest, payload, null, null, null, null); msg.setPayload(data); @@ -352,7 +363,12 @@ class I2CPMessageProducer { msg.setLeaseSet(leaseSet); msg.setPrivateKey(priv); msg.setSigningPrivateKey(signingPriv); - msg.setSessionId(session.getSessionId()); + SessionId sid = session.getSessionId(); + if (sid == null) { + _log.error(session.toString() + " create LS w/o session", new Exception()); + return; + } + msg.setSessionId(sid); session.sendMessage(msg); } @@ -381,7 +397,12 @@ class I2CPMessageProducer { throw new I2PSessionException("Unable to sign the session config", dfe); } msg.setSessionConfig(cfg); - msg.setSessionId(session.getSessionId()); + SessionId sid = session.getSessionId(); + if (sid == null) { + _log.error(session.toString() + " update config w/o session", new Exception()); + return; + } + msg.setSessionId(sid); session.sendMessage(msg); } } diff --git a/core/java/src/net/i2p/client/I2PSessionImpl.java b/core/java/src/net/i2p/client/I2PSessionImpl.java index bc34f88f0..6532b1f9f 100644 --- a/core/java/src/net/i2p/client/I2PSessionImpl.java +++ b/core/java/src/net/i2p/client/I2PSessionImpl.java @@ -502,6 +502,8 @@ public abstract class I2PSessionImpl implements I2PSession, I2CPMessageReader.I2 } protected void changeState(State state) { + if (_log.shouldInfo()) + _log.info(getPrefix() + "Change state to " + state); synchronized (_stateLock) { _state = state; _stateLock.notifyAll(); @@ -891,8 +893,9 @@ public abstract class I2PSessionImpl implements I2PSession, I2CPMessageReader.I2 public void messageReceived(I2CPMessageReader reader, I2CPMessage message) { int type = message.getType(); SessionId id = message.sessionId(); - if (id == null || id.equals(_sessionId) || - (_sessionId == null && id != null && type == SessionStatusMessage.MESSAGE_TYPE)) { + SessionId currId = _sessionId; + if (id == null || id.equals(currId) || + (currId == null && id != null && type == SessionStatusMessage.MESSAGE_TYPE)) { // it's for us I2CPMessageHandler handler = _handlerMap.getHandler(type); if (handler != null) { @@ -1123,6 +1126,13 @@ public abstract class I2PSessionImpl implements I2PSession, I2CPMessageReader.I2 locked_closeSocket(); changeState(State.CLOSED); } + synchronized (_subsessionLock) { + for (SubSession sess : _subsessions) { + sess.changeState(State.CLOSED); + sess.setSessionId(null); + sess.setLeaseSet(null); + } + } } /** @@ -1152,6 +1162,8 @@ public abstract class I2PSessionImpl implements I2PSession, I2CPMessageReader.I2 _socket = null; // so when propogateError calls closeSocket, it doesnt loop } } + setSessionId(null); + setLeaseSet(null); } /** @@ -1238,8 +1250,10 @@ public abstract class I2PSessionImpl implements I2PSession, I2CPMessageReader.I2 buf.append(s); else buf.append(getClass().getSimpleName()); - if (_sessionId != null) - buf.append(" #").append(_sessionId.getSessionId()); + SessionId id = _sessionId; + if (id != null) + buf.append(" #").append(id.getSessionId()); + buf.append(' ').append(_state.toString()); buf.append("]: "); return buf.toString(); } diff --git a/history.txt b/history.txt index f43fa1b52..47c310559 100644 --- a/history.txt +++ b/history.txt @@ -8,6 +8,10 @@ Prop from i2p.i2p.zzz.multisess: - Add support for 'aliased' local destinations that use the same tunnel pools - No UI or config support, no server support, may be added later - Catch uncaught exceptions in ClientConnectionRunner and stop connection + - When socket is closed, set sessionID and LS to null, + close subsession and set its sessionID and LS to null + - Checks on client side for null session ID + - Check for null session in Destroy Session message 2015-06-13 zzz * i2psnark: Fix NPE (ticket #1602) diff --git a/router/java/src/net/i2p/router/client/ClientMessageEventListener.java b/router/java/src/net/i2p/router/client/ClientMessageEventListener.java index d2a6ceeb0..f0b273c4a 100644 --- a/router/java/src/net/i2p/router/client/ClientMessageEventListener.java +++ b/router/java/src/net/i2p/router/client/ClientMessageEventListener.java @@ -8,6 +8,7 @@ package net.i2p.router.client; * */ +import java.util.List; import java.util.Properties; import net.i2p.CoreVersion; @@ -30,6 +31,7 @@ import net.i2p.data.i2cp.I2CPMessageException; import net.i2p.data.i2cp.I2CPMessageReader; import net.i2p.data.i2cp.MessageId; import net.i2p.data.i2cp.MessagePayloadMessage; +import net.i2p.data.i2cp.MessageStatusMessage; import net.i2p.data.i2cp.ReceiveMessageBeginMessage; import net.i2p.data.i2cp.ReceiveMessageEndMessage; import net.i2p.data.i2cp.ReconfigureSessionMessage; @@ -365,9 +367,28 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi SessionId sid = message.getSessionId(); SessionConfig cfg = _runner.getConfig(sid); if (cfg == null) { + List current = _runner.getSessionIds(); + String msg = "SendMessage invalid session: " + sid + " current: " + current; if (_log.shouldLog(Log.ERROR)) - _log.error("SendMessage w/o session: " + sid); - _runner.disconnectClient("SendMessage w/o session: " + sid); + _log.error(msg); + // Just drop the message for now, don't kill the whole socket... + // bugs on client side, esp. prior to 0.9.21, may cause sending + // of messages before the session is established + //_runner.disconnectClient(msg); + // do this instead: + if (sid != null && message.getNonce() > 0) { + MessageStatusMessage status = new MessageStatusMessage(); + status.setSessionId(sid.getSessionId()); + status.setSize(0); + status.setNonce(message.getNonce()); + status.setStatus(MessageStatusMessage.STATUS_SEND_FAILURE_BAD_SESSION); + try { + _runner.doSend(status); + } catch (I2CPMessageException ime) { + if (_log.shouldLog(Log.WARN)) + _log.warn("Error writing out the message status message", ime); + } + } return; } if (_log.shouldLog(Log.DEBUG)) @@ -424,10 +445,14 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi private void handleDestroySession(DestroySessionMessage message) { SessionId id = message.getSessionId(); - SessionConfig cfg = _runner.getConfig(id); - _runner.removeSession(id); + if (id != null) { + _runner.removeSession(id); + } else { + if (_log.shouldLog(Log.WARN)) + _log.warn("destroy session with null ID"); + } int left = _runner.getSessionIds().size(); - if (left <= 0) { + if (left <= 0 || id == null) { _runner.stopRunning(); } else { if (_log.shouldLog(Log.INFO)) @@ -446,9 +471,11 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi SessionId id = message.getSessionId(); SessionConfig cfg = _runner.getConfig(id); if (cfg == null) { + List current = _runner.getSessionIds(); + String msg = "CreateLeaseSet invalid session: " + id + " current: " + current; if (_log.shouldLog(Log.ERROR)) - _log.error("CreateLeaseSet w/o session: " + id); - _runner.disconnectClient("CreateLeaseSet w/o session: " + id); + _log.error(msg); + _runner.disconnectClient(msg); return; } Destination dest = cfg.getDestination(); @@ -536,10 +563,12 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi SessionId id = message.getSessionId(); SessionConfig cfg = _runner.getConfig(id); if (cfg == null) { + List current = _runner.getSessionIds(); + String msg = "ReconfigureSession invalid session: " + id + " current: " + current; if (_log.shouldLog(Log.ERROR)) - _log.error("ReconfigureSession w/o session: " + id); + _log.error(msg); //sendStatusMessage(id, SessionStatusMessage.STATUS_INVALID); - _runner.disconnectClient("ReconfigureSession w/o session: " + id); + _runner.disconnectClient(msg); return; } if (_log.shouldLog(Log.INFO))