Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check_http -e breaks -f #91

Closed
cp0815 opened this issue Jul 29, 2015 · 29 comments
Closed

check_http -e breaks -f #91

cp0815 opened this issue Jul 29, 2015 · 29 comments

Comments

@cp0815
Copy link

cp0815 commented Jul 29, 2015

It took me some time a an longer look to the source code to find out, that using the -e option breaks the -f option.
I checked this in the source code of the plugins we are currently using (1.4.15) and also on the newest version 2.0.3
In 2.0.3 the relevant code starts in line 1073:
if ( server_expect_yn ) {
...
} else {
...
only the else block contains the handling for the redirects.
It would by very nice if this would be fixed in future versions

@tmcnag
Copy link
Contributor

tmcnag commented Jul 29, 2015

Can you expand a bit? What specifically do you mean by -e "breaks" -f? Can you describe the problem or post the output vs what is expected?

@scottwilkerson
Copy link

I am curious also, the help states the following for -e
If specified skips all other status line logic (ex: 3xx, 4xx, 5xx processing)

@cp0815
Copy link
Author

cp0815 commented Jul 29, 2015

Sorry, when i was to short.
The -f option allows to advise the check_http plugin to follow a redirect.
So if the url that is checked answers with a redirect (http code 30x) to another page,
the check_http plugin follows this redirect and checks the page where the redirect points to (http status code of this page, also regex checks and so on...)
but when using a -e parameter (f.e. -e '200,302) the redirect is not followed anymore.
took me some time to find out why the -f is not working, after reading the source I found out, that this is because of the -e.
And yes, the help mentions that -e skips all status line logic, but I didn't expect that this also matches to the redirect parameter

@scottwilkerson
Copy link

Being the fact that when you use the -e option it is just determining if the first (status) line of the server response (default: HTTP/1.) matches your list, that response is returned with all requests so the first request id what should be used.

If you use the -e flag, it will not do regex matches even if you aren't redirected

e.g.
./check_http -H www.nagios.com -S -r nagios -e 200
HTTP OK: Status line output matched "200" - 57997 bytes in 0.394 second response time |time=0.393674s;;;0.000000 size=57997B;;;0

without -e
./check_http -H www.nagios.com -S -r nagios
HTTP OK: HTTP/1.1 200 OK - 57997 bytes in 0.605 second response time |time=0.605441s;;;0.000000 size=57997B;;;0

The -e flag forces to just do a --expect match on the first (status) line of the server response

@abrist
Copy link

abrist commented Jul 29, 2015

Scott is correct - this is working as expected. Would you like to see the behavior altered in the future?

@cp0815
Copy link
Author

cp0815 commented Jul 29, 2015

Sorry, this is not correct. Even if the standard output doesn't show this, it does.
You can simply test this if you do this:
./check_http -H www.nagios.com -S -r nagios123 -e 200
HTTP CRITICAL: Status line output matched "200" - pattern not found - 57997 bytes in 0.877 second response time |time=0.877121s;;;0.000000 size=57997B;;;0

@abrist
Copy link

abrist commented Jul 29, 2015

This is due to the inclusion of an unmatched regex statement: -r nagios123
This is generating the "pattern not found" error message.

Remove the regex check and it works:

$ ./plugins/check_http -H www.nagios.com -S -e 200
HTTP OK: Status line output matched "200" - 58028 bytes in 0.277 second response time |time=0.277178s;;;0.000000 size=58028B;;;0

Or change the regex to a pattern that matches and it works:

$ ./plugins/check_http -H www.nagios.com -S -r nagios -e 200
HTTP OK: Status line output matched "200" - 58028 bytes in 0.301 second response time |time=0.301496s;;;0.000000 size=58028B;;;0

Some of the confusion here could be that the "pattern not found" error is not that verbose. We could at least change it to "regex pattern not found" - would that help? I do not think we want to reprint the failed regex string though as some users have fairly lengthy regex patterns they check for and it would muddy the plugin status output.

@cp0815
Copy link
Author

cp0815 commented Jul 29, 2015

Sorry, maybe my answer was again to short.
Scott wrote:
If you use the -e flag, it will not do regex matches even if you aren't redirected

So I wanted to show, that even when using the -e parameter a regex match is done.
The check that is using this wrong regex "nagios123" is going critical, because the regex check is done even when the -e option is used.
So imho nobody expects, that the -f follow option is not working when using the -r and -e options.
Also i would be the only option to check the status code of the final page for a specific code (f.e. -e '201,302' )
Therefore in my opinion this should either be mentioned in in help and man textes or should be fixed with a small fix that enables the use of the -f option even then a -e is defined. This should be very easy in the code.

@scottwilkerson
Copy link

I'm going to come back around and retract my earlier consensus, I like @cp0815 proposal, I believe the plugin should be modified to perform the -f option (if specified) and then do the checking on the page.

@abrist
Copy link

abrist commented Jul 29, 2015

I agree the intended behavior should match the expected behavior. I will try to look at this tomorrow with John.

@abrist
Copy link

abrist commented Jul 30, 2015

After looking at the code and doing some testing, it seems like -e does not disable -f in the 2.1.0-beta-RC1 branch, but it does break the ability to run a regex against the redirected page:

$ ./plugins/check_http -H nagios.com
HTTP OK: HTTP/1.1 301 Moved Permanently - 522 bytes in 0.094 second response time |time=0.093743s;;;0.000000 size=522B;;;0

$ ./plugins/check_http -H nagios.com -r standard
HTTP CRITICAL: HTTP/1.1 301 Moved Permanently - pattern not found - 522 bytes in 0.089 second response time |time=0.088574s;;;0.000000 size=522B;;;0

$ ./plugins/check_http -H nagios.com -r standard -f follow
HTTP OK: HTTP/1.1 200 OK - 58039 bytes in 0.880 second response time |time=0.880250s;;;0.000000 size=58039B;;;0

$ ./plugins/check_http -H nagios.com -r standard -f follow -e 301
HTTP CRITICAL: Status line output matched "301" - pattern not found - 522 bytes in 0.100 second response time |time=0.099612s;;;0.000000 size=522B;;;0

$ ./plugins/check_http -H nagios.com -f follow -e 301
HTTP OK: Status line output matched "301" - 522 bytes in 0.089 second response time |time=0.089001s;;;0.000000 size=522B;;;0

I will look at this some more.

@abrist
Copy link

abrist commented Jul 30, 2015

This can actually be a bit complicated when dealing with follow on redirects. When -e and -f follow are both specified, what should the behavior of -e be? Should it check against the first server contacted (the 301) or against the last server in the case of multiple redirects to -f follow through?

@tmcnag
Copy link
Contributor

tmcnag commented Jul 30, 2015

My expectation would be the last server when -f is supplied, otherwise the first.

@scottwilkerson
Copy link

I'm with @tmcnag on this. Of course it would be impossible to match a 301 or 302 with the -e, but that would be implied.

@tmcnag
Copy link
Contributor

tmcnag commented Jul 30, 2015

We might want to modify -f to only follow so many redirects (configurable), both to prevent endless runs in case of circular redirects, and to all -e to have a hard endpoint so that you can match a 3XX status.

@scottwilkerson
Copy link

The plugin does have a max_depth of 15, so you are correct, the -e would match the 3XX in a circular loop, or anytime 15+ redirects happen.

@tmcnag
Copy link
Contributor

tmcnag commented Jul 30, 2015

Shouldn't be too difficult at all to make that configurable then.

@scottwilkerson
Copy link

Correction, it will not match the -e, but will return:
HTTP WARNING - maximum redirection depth 15 exceeded

@scottwilkerson
Copy link

Yes, it certainly could be made a config option, but should still default to 15

@cp0815
Copy link
Author

cp0815 commented Jul 30, 2015

I don't know what would be the best. It is hard to decide for all possible kinds to use the check and to know what all possbile application logics are, that could have to be monitored.
I think about the option to simply check all status codes. This was my first expectation and it would give the option to check both, the status codes of the redirects (to check if these are the expected ones: temporary or permantent, or specify both it doesn't make a difference) and the status code of the final page. As the -e option already allows to specify more than one status code, there is no change required for this.
I didn't check back to the source of the RC but as far as I remember from the source of the older versions this would be very easy to implement, maybe more easy then to check the final status code.
Does this make any sense?

Am 30. Juli 2015 22:27:50 MESZ, schrieb abrist notifications@github.com:

This can actually be a bit complicated when dealing with follow on
redirects. When -e and -f follow are both specified, what should the
behavior of -e be? Should it check against the first server contacted
(the 301) or against the last server in the case of multiple redirects
to -f follow through?


Reply to this email directly or view it on GitHub:
#91 (comment)

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

@abrist
Copy link

abrist commented Jul 30, 2015

I will look into this more tomorrow - I have a few different branches that get close to giving you what you want.

@abrist
Copy link

abrist commented Sep 1, 2015

My original ideas caused more issues than they solved. And while they clarified the behavior of -f when -e is supplied, they created ambiguity when just checking redirects.

@kkaczmarczyk
Copy link

Just checking - is this improvement still in plan? I would love to have that, and I couldn't find any work-arounds

@rockyba
Copy link

rockyba commented Mar 31, 2016

I also would really like this improvement, was this improvement added in a newer version or is it still in the works?

jfrickson pushed a commit that referenced this issue Nov 17, 2016
Fix for issue #91

Checking status codes and following redirects makes for compicated
coding and testing. But this change seems to handle all situations
properly.

It is important to note, though, that anything specified by `-e`
will only be compared agains the final page. So if you use `-e 301`
expecting a redirect, it will error out because the status of the
last page would be `200` (or some other status).
@jfrickson
Copy link
Contributor

Probable fix in branch 'maint' via commit 657e3b6.

Please test and report successes/failures/problems.

@jfrickson
Copy link
Contributor

Fixed in branch 'maint' via commit 657e3b6

@heaje
Copy link

heaje commented Dec 9, 2016

@jfrickson: I found that this particular patch to 2.1.4 breaks -e logic with a 401. For instance, I run this:

./check_http -I ip.that.should.401 -S -e 401

and I got this returned:

HTTP WARNING: Status line output matched "401" - HTTP/1.1 401 Unauthorized - 164 bytes in 0.935 second response time |time=0.935251s;;;0.000000 size=164B;;;0

I should have gotten an OK (I did put -e 401), but it still came back as a warning.

I also tried just building check_http from the branch 'maint' (maybe something there fixed it), but that failed with the same issue.

@greg17477
Copy link

As heaje stated, even if the expected (-e) output line/string is found, the check returns WARNING or CRITICAL instead of OK.

@mat813
Copy link
Contributor

mat813 commented Feb 9, 2017

I have the same problem, I have an HTTPS check that uses check_http -H blah -S -e HTTP/1.1 and before this it did not care about the status code, just that there was a https server behind. Now, most of my HTTPS checks are in a WARNING state because the host used with -H does not match a vhost, so I get 400/401/403 errors, which are just fine. Also, I cannot use -e 403 to check that I get a 403 any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants