diff --git a/router/java/src/net/i2p/router/transport/UPnP.java b/router/java/src/net/i2p/router/transport/UPnP.java index 1c9472150..8fe45ee41 100644 --- a/router/java/src/net/i2p/router/transport/UPnP.java +++ b/router/java/src/net/i2p/router/transport/UPnP.java @@ -12,7 +12,6 @@ import java.util.Set; import net.i2p.util.Log; import net.i2p.I2PAppContext; -import net.i2p.router.RouterContext; import org.cybergarage.upnp.Action; import org.cybergarage.upnp.ActionList; @@ -94,6 +93,8 @@ public class UPnP extends ControlPoint implements DeviceChangeListener { public void terminate() { unregisterPortMappings(); super.stop(); + _router = null; + _service = null; } public DetectedIP[] getAddress() { @@ -140,10 +141,13 @@ public class UPnP extends ControlPoint implements DeviceChangeListener { return; } } - if(!ROUTER_DEVICE.equals(dev.getDeviceType()) || !dev.isRootDevice()) - return; // Silently ignore non-IGD devices - else if(isNATPresent()) { - _log.error("We got a second IGD on the network! the plugin doesn't handle that: let's disable it."); + if(!ROUTER_DEVICE.equals(dev.getDeviceType()) || !dev.isRootDevice()) { + _log.warn("UP&P non-IGD device found, ignoring : " + dev.getFriendlyName()); + return; // ignore non-IGD devices + } else if(isNATPresent()) { + // maybe we should see if the old one went away before ignoring the new one? + _log.warn("UP&P ignoring additional IGD device found: " + dev.getFriendlyName() + " UDN: " + dev.getUDN()); + /********** seems a little drastic isDisabled = true; synchronized(lock) { @@ -152,18 +156,21 @@ public class UPnP extends ControlPoint implements DeviceChangeListener { } stop(); + **************/ return; } - _log.warn("UP&P IGD found : " + dev.getFriendlyName()); + _log.warn("UP&P IGD found : " + dev.getFriendlyName() + " UDN: " + dev.getUDN()); synchronized(lock) { _router = dev; } discoverService(); // We have found the device we need: stop the listener thread - stop(); + /// No, let's stick around to get notifications + //stop(); synchronized(lock) { + /// we should look for the next one if(_service == null) { _log.error("The IGD device we got isn't suiting our needs, let's disable the plugin"); isDisabled = true; @@ -238,15 +245,29 @@ public class UPnP extends ControlPoint implements DeviceChangeListener { } public void deviceRemoved(Device dev ){ + _log.warn("UP&P device removed : " + dev.getFriendlyName() + " UDN: " + dev.getUDN()); synchronized (lock) { if(_router == null) return; - if(_router.equals(dev)) { + // I2P this wasn't working + //if(_router.equals(dev)) { + if(ROUTER_DEVICE.equals(dev.getDeviceType()) && + dev.isRootDevice() && + stringEquals(_router.getFriendlyName(), dev.getFriendlyName()) && + stringEquals(_router.getUDN(), dev.getUDN())) { + _log.warn("UP&P IGD device removed : " + dev.getFriendlyName()); _router = null; _service = null; } } } + /** compare two strings, either of which could be null */ + private static boolean stringEquals(String a, String b) { + if (a != null) + return a.equals(b); + return b == null; + } + /** * @return whether we are behind an UPnP-enabled NAT/router */ @@ -266,7 +287,11 @@ public class UPnP extends ControlPoint implements DeviceChangeListener { if(getIP == null || !getIP.postControlAction()) return null; - return (getIP.getOutputArgumentList().getArgument("NewExternalIPAddress")).getValue(); + String rv = (getIP.getOutputArgumentList().getArgument("NewExternalIPAddress")).getValue(); + // I2P some devices return 0.0.0.0 when not connected + if ("0.0.0.0".equals(rv)) + return null; + return rv; } /** @@ -416,7 +441,11 @@ public class UPnP extends ControlPoint implements DeviceChangeListener { // FIXME L10n! sb.append("

Found "); listSubDev(null, _router, sb); - sb.append("
The current external IP address reported by UPnP is " + getNATAddress()); + String addr = getNATAddress(); + if (addr != null) + sb.append("
The current external IP address reported by UPnP is " + addr); + else + sb.append("
The current external IP address is not available."); int downstreamMaxBitRate = getDownstreamMaxBitRate(); int upstreamMaxBitRate = getUpstramMaxBitRate(); if(downstreamMaxBitRate > 0) @@ -554,36 +583,78 @@ public class UPnP extends ControlPoint implements DeviceChangeListener { return "?"; } + private static int __id = 0; + + /** + * postControlAction() can take many seconds, especially if it's failing, + * and onChangePublicPorts() may be called from threads we don't want to slow down, + * so throw this in a thread. + */ private void registerPorts(Set portsToForwardNow) { - for(ForwardPort port : portsToForwardNow) { - String proto = protoToString(port.protocol); - if (proto.length() <= 1) { - HashMap map = new HashMap(); - map.put(port, new ForwardPortStatus(ForwardPortStatus.DEFINITE_FAILURE, "Protocol not supported", port.portNumber)); - forwardCallback.portForwardStatus(map); - continue; - } - if(tryAddMapping(proto, port.portNumber, port.name, port)) { - HashMap map = new HashMap(); - map.put(port, new ForwardPortStatus(ForwardPortStatus.MAYBE_SUCCESS, "Port apparently forwarded by UPnP", port.portNumber)); - forwardCallback.portForwardStatus(map); - continue; - } else { - HashMap map = new HashMap(); - map.put(port, new ForwardPortStatus(ForwardPortStatus.PROBABLE_FAILURE, "UPnP port forwarding apparently failed", port.portNumber)); - forwardCallback.portForwardStatus(map); - continue; + Thread t = new Thread(new RegisterPortsThread(portsToForwardNow)); + t.setName("UPnP Port Opener " + (++__id)); + t.setDaemon(true); + t.start(); + } + + private class RegisterPortsThread implements Runnable { + private Set portsToForwardNow; + + public RegisterPortsThread(Set ports) { + portsToForwardNow = ports; + } + + public void run() { + for(ForwardPort port : portsToForwardNow) { + String proto = protoToString(port.protocol); + if (proto.length() <= 1) { + HashMap map = new HashMap(); + map.put(port, new ForwardPortStatus(ForwardPortStatus.DEFINITE_FAILURE, "Protocol not supported", port.portNumber)); + forwardCallback.portForwardStatus(map); + continue; + } + if(tryAddMapping(proto, port.portNumber, port.name, port)) { + HashMap map = new HashMap(); + map.put(port, new ForwardPortStatus(ForwardPortStatus.MAYBE_SUCCESS, "Port apparently forwarded by UPnP", port.portNumber)); + forwardCallback.portForwardStatus(map); + continue; + } else { + HashMap map = new HashMap(); + map.put(port, new ForwardPortStatus(ForwardPortStatus.PROBABLE_FAILURE, "UPnP port forwarding apparently failed", port.portNumber)); + forwardCallback.portForwardStatus(map); + continue; + } } } } + /** + * postControlAction() can take many seconds, especially if it's failing, + * and onChangePublicPorts() may be called from threads we don't want to slow down, + * so throw this in a thread. + */ private void unregisterPorts(Set portsToForwardNow) { - for(ForwardPort port : portsToForwardNow) { - String proto = protoToString(port.protocol); - if (proto.length() <= 1) - // Ignore, we've already complained about it - continue; - removeMapping(proto, port.portNumber, port, false); + Thread t = new Thread(new UnregisterPortsThread(portsToForwardNow)); + t.setName("UPnP Port Opener " + (++__id)); + t.setDaemon(true); + t.start(); + } + + private class UnregisterPortsThread implements Runnable { + private Set portsToForwardNow; + + public UnregisterPortsThread(Set ports) { + portsToForwardNow = ports; + } + + public void run() { + for(ForwardPort port : portsToForwardNow) { + String proto = protoToString(port.protocol); + if (proto.length() <= 1) + // Ignore, we've already complained about it + continue; + removeMapping(proto, port.portNumber, port, false); + } } } diff --git a/router/java/src/org/cybergarage/http/HTTPServer.java b/router/java/src/org/cybergarage/http/HTTPServer.java index 67f3f1a21..1e44b6f8b 100644 --- a/router/java/src/org/cybergarage/http/HTTPServer.java +++ b/router/java/src/org/cybergarage/http/HTTPServer.java @@ -174,7 +174,7 @@ public class HTTPServer implements Runnable Thread.yield(); Socket sock; try { - Debug.message("accept ..."); + //Debug.message("accept ..."); sock = accept(); if (sock != null) Debug.message("sock = " + sock.getRemoteSocketAddress()); @@ -185,7 +185,7 @@ public class HTTPServer implements Runnable } HTTPServerThread httpServThread = new HTTPServerThread(this, sock); httpServThread.start(); - Debug.message("httpServThread ..."); + //Debug.message("httpServThread ..."); } } diff --git a/router/java/src/org/cybergarage/upnp/device/Disposer.java b/router/java/src/org/cybergarage/upnp/device/Disposer.java index e6c0ddb8e..1de8ceb3b 100644 --- a/router/java/src/org/cybergarage/upnp/device/Disposer.java +++ b/router/java/src/org/cybergarage/upnp/device/Disposer.java @@ -51,6 +51,7 @@ public class Disposer extends ThreadCore public void run() { + Thread.currentThread().setName("UPnP-Disposer"); ControlPoint ctrlp = getControlPoint(); long monitorInterval = ctrlp.getExpiredDeviceMonitoringInterval() * 1000; diff --git a/router/java/src/org/cybergarage/upnp/ssdp/SSDPNotifySocket.java b/router/java/src/org/cybergarage/upnp/ssdp/SSDPNotifySocket.java index 6a33af7c3..75484bd16 100644 --- a/router/java/src/org/cybergarage/upnp/ssdp/SSDPNotifySocket.java +++ b/router/java/src/org/cybergarage/upnp/ssdp/SSDPNotifySocket.java @@ -104,7 +104,8 @@ public class SSDPNotifySocket extends HTTPMUSocket implements Runnable InetAddress maddr = getMulticastInetAddress(); InetAddress pmaddr = packet.getHostInetAddress(); if (maddr.equals(pmaddr) == false) { - Debug.warning("Invalidate Multicast Recieved : " + maddr + "," + pmaddr); + // I2P + //Debug.warning("Invalidate Multicast Recieved : " + maddr + "," + pmaddr); continue; } diff --git a/router/java/src/org/cybergarage/xml/Parser.java b/router/java/src/org/cybergarage/xml/Parser.java index d93cf48a2..2c9b3861b 100644 --- a/router/java/src/org/cybergarage/xml/Parser.java +++ b/router/java/src/org/cybergarage/xml/Parser.java @@ -3,7 +3,7 @@ * CyberXML for Java * * Copyright (C) Satoshi Konno 2002 -* +* * File: Parser.java * * Revision; @@ -14,14 +14,14 @@ * - Change parse(String) to use StringBufferInputStream instead of URL. * ******************************************************************/ - + package org.cybergarage.xml; import java.net.*; import java.io.*; public abstract class Parser -{ +{ //////////////////////////////////////////////// // Constructor //////////////////////////////////////////////// @@ -44,6 +44,13 @@ public abstract class Parser { try { HttpURLConnection urlCon = (HttpURLConnection)locationURL.openConnection(); + // I2P mods to prevent hangs (see HTTPRequest for more info) + // this seems to work, getInputStream actually does the connect(), + // (as shown by a thread dump) + // so we can set these after openConnection() + // Alternative would be foo = new HttpURLConnection(locationURL); foo.set timeouts; foo.connect() + urlCon.setConnectTimeout(2*1000); + urlCon.setReadTimeout(1000); urlCon.setRequestMethod("GET"); InputStream urlIn = urlCon.getInputStream();