[Nagiosplug-devel] Re: Latest snapshot check_http

[sean finney]

hi emmet,

i'm cc'ing this to the nagiosplug-devel list, hope that's okay.

On Fri, Mar 17, 2006 at 11:53:35AM -0500, Hogan, Emmett (LNG-SAC) wrote:
> I just downloaded the latest nagios_plugins snapshot to get the updated
> check_http.  I was having some issues with checking my servers which
> required STRONG encryption.  I am *very* happy to say that your latest
> updates to the plugin fixed my problem, but I came across one very small
> bug.

that's good to hear, minus the bug :)

> If you are checking the certificate, and the certificate is valid,
> check_http still performs the full http check and returns BOTH results:
> 
> # ./check_http --ssl -C 14 www.verisign.com
> OK - Certificate will expire on 06/01/2007 23:59.
> HTTP OK HTTP/1.1 200 OK - 29243 bytes in 4.414 seconds
> |time=4.414328s;;;0.000000 size=29243B;;;0
> 
> This causes problems if the cert is ok, but the page you check is not. :-)

aha.  i forgot that -C explicitly states that it checks the cert
only, and not the page.

> Anyway, I am *not* a C coder, but it looked to me like there is a return in
> the wrong place:
> 
> *** check_http.c.DIST   2006-03-17 08:20:30.000000000 -0800
> --- check_http.c.mine   2006-03-17 08:23:07.000000000 -0800
> ***************
> *** 758,765 ****
>         if(result != STATE_OK){
>           np_net_ssl_cleanup();
>           if(sd) close(sd);
> -         return result;
>         }
>       }
>     }
>   #endif /* HAVE_SSL */
> --- 758,765 ----
>         if(result != STATE_OK){
>           np_net_ssl_cleanup();
>           if(sd) close(sd);
>         }
> +       return result;
>       }
>     }
>   #endif /* HAVE_SSL */

this is close, but i think the proper fix is to move all three lines
inside the if statement outside, and then remove the iff statement
entirely.  if we're checking the cert, after doing so we should
cleanup and return the result regardless of its value.  but good
catch!  like i said i managed to forget how -C was supposed to
behave.

> If this was the wrong thing to do to fix this, please let me know. :-)
> 
> Thanks a lot for all your work on the plugins and especially for fixing the
> SSL issues I was having!!! :-)

no prob... i've just made a commit that should fix this.

on a related note: i notice that -C does not imply --ssl, but the
documentation for check_http says

	check_http www.verisign.com -C 14

should work, but it doesn't unless you provide --ssl.  should the
documentation or the code be changed?


	sean
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <https://www.monitoring-plugins.org/archive/devel/attachments/20060318/83148ec3/attachment.sig>