sky201503 / openhab

Automatically exported from code.google.com/p/openhab
GNU General Public License v3.0
0 stars 0 forks source link

Network health binding should use another approach to determine if a host is reachable or not #340

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
1. Feature Description

The current implementation of the network health binding is using the 
isReachable() method of the InetAddress class.
The problem is that this method does not guarantee to work 100% correctly or 
better say the user can not always expect to get the right result. 
Often it depends on the used timeout or the missing privileges to use ICMP. In 
fact isReachable() seems not to be equivalent to a real ping. If you take a 
look at the documentation it says depending on permissions it will try to 
connect to the TCP port 7 or do an ICMP request. If you can't create raw 
sockets, the isReachable() method can't use ICMP
and therefore it attempts to connect to port 7. This results in a "connection 
refused" which means that the machine is there but refuses connection on that 
particular port.

What I have done so far is using another approach to test if a URL is availabe 
or not. For this I implemented my own isReachable() method (see below) in the 
org.openhab.io.net.actions.Ping class and replaced the line 

success = InetAddress.getByName(host).isReachable(timeout); 

with

success = isReachable(timeout);

Please note that this approach requires that full qualified URL'are used which 
mean that you have to use for example http://www.openhab.org instead 
of openhab.org. Doing this it will not be possible to use : as separator 
because the split would deliver wrong results.
So another small modification needs to be done in 
NetworkHealthGenericBindingProvider.processBindingConfiguration(). Just change 
the used delimeter from : to |. 

Before:
String[] configParts = bindingConfig.trim().split(":");

After:
String[] configParts = bindingConfig.trim().split("|");

Now you can use the following binding configuration string:

Before:
nh="openhab.org:443:2000"

After:
nh="openhab.org|443|2000"

/**
 * Will check if the given (full qualified) URL is reachable
 * @param fullQualifiedUrl The full qualified URL (for example http://www.openhab.org) to be checked
 * @param timeout The connection timeout to be used
 * @return true if the full qualified URL is reachable else false
 */
private static String isReachable(final String fullQualifiedUrl,final int 
timeout)
{
    String url = fullQualifiedUrl;
    try 
    {
      HttpURLConnection.setFollowRedirects(false);

      //TODO: maybe HttpURLConnection.setInstanceFollowRedirects(false) is also needed
      HttpURLConnection con =(HttpURLConnection) new URL(url).openConnection();
      con.setRequestMethod("HEAD");
      con.setConnectTimeout(timeout);
      con.setReadTimeout(timeout);

      //check if the URL has moved and point to that new URL
      if(con.getResponseCode() == HttpURLConnection.HTTP_MOVED_TEMP  || con.getResponseCode() == HttpURLConnection.HTTP_MOVED_PERM)
      {
          url = con.getHeaderField("Location");
          con =(HttpURLConnection) new URL(url).openConnection();
          con.setConnectTimeout(timeout);
          con.setReadTimeout(timeout);
          con.setRequestMethod("HEAD");
      }

      //TODO: analyze if we need to check also for further return codes (202 Accepted, 203 Non-Authoritative Information,204 No Content,......)
      //check for 200 (OK) and also for 403 (FORBIDDEN) -> nevertheless we receive the 403 response the URL is reachable
      if(con.getResponseCode() == HttpURLConnection.HTTP_OK || con.getResponseCode() == HttpURLConnection.HTTP_FORBIDDEN)
          return true;

      return false;

    }
    catch(SocketTimeoutException exc)
    {
        //TODO: log
        return false;
    }
    catch (ConnectException ce) 
    {
        //TODO: log
        return false;
    }
    catch (Exception e) 
    {
        //TODO: log
        return false;
    }
}

Original issue reported on code.google.com by dongy...@umc-project.de on 12 Jun 2013 at 9:49

GoogleCodeExporter commented 8 years ago
The return type of the isReachable method should be indeed boolean and not 
String!

Original comment by dongy...@umc-project.de on 12 Jun 2013 at 9:53

GoogleCodeExporter commented 8 years ago
The problem I see here is that you can only test hosts with a http server. But 
what others hosts?

Original comment by till.klo...@gmail.com on 14 Jun 2013 at 10:22

GoogleCodeExporter commented 8 years ago

Original comment by teichsta on 5 Nov 2013 at 10:47

GoogleCodeExporter commented 8 years ago
This issue has been migrated to Github. If this issue id is greater than103 its 
id has been preserved on Github. You can open your issue by calling the URL 
https://github.com/openhab/openhab/issues/<issueid>. Issues with ids less or 
equal 103 new ids were created.

Original comment by teichsta on 17 Nov 2013 at 8:08

GoogleCodeExporter commented 8 years ago
see above!

Issue has been migrated to Github and should be discussed there.

Original comment by teichsta on 21 Nov 2013 at 1:51