-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add support for content match in HTTP searches #136
Conversation
Anyone knows why the build has failed?
It seems to be some missing env variable. |
Hm, not sure. The tests all passed per https://travis-ci.org/etsy/411/builds/270582655#L636. Will review the code next week, thanks for your PR. :] |
phplib/Search/HTTP.php
Outdated
return true; | ||
} | ||
|
||
return (bool)preg_match("/" . $regex . "/", $response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be !== false
. See https://secure.php.net/manual/en/function.preg-match.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
@@ -6,3 +6,8 @@ | |||
<label for="code">Code</label> | |||
<input class="form-control" type="number" name="code" value="{{ query_data.code }}" /> | |||
</div> | |||
|
|||
<div class="col-xs-12 col-sm-12 form-group"> | |||
<label for="code">Content Match <span class="glyphicon glyphicon-question-sign" data-toggle="tooltip" title="This regular expression will run against the raw response content, leave it blank for using only status code check."></span></label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight tweak: This regular expression will run against the raw response body. Leave blank to disable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better :)
phplib/Search/HTTP.php
Outdated
$alert['content'] = [ | ||
'status' => $curl->httpStatusCode, | ||
'url' => $constructed_qdata['url'], | ||
'content length' => strlen($curl->rawResponse) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
content_length
would be preferable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
Hey, @kiwiz, thank you for reviewing my code, I did all the adjustments that you commented and I also have updated the documentation adding the new field. |
Merged, thanks for the contribution! |
Hello guys,
As I've mentioned in issue #116, I implemented a new field in the HTTP search type to add a validation based on the response content using a regular expression to match when the status code is not enough.
This is an optional field, so you can leave it blank to validation only the status code.
I also have added a new information in the alert to show the content length for that given request.
We may need to think in a better way to persist the response content to help to understand the problem when it not matches.