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

[dev.icinga.com #10366] Text plugin output treated as HTML in too many occasions #2085

Closed
icinga-migration opened this issue Oct 15, 2015 · 4 comments

Comments

Projects
None yet
1 participant
@icinga-migration
Copy link
Member

commented Oct 15, 2015

This issue has been migrated from Redmine: https://dev.icinga.com/issues/10366

Created by tgelf on 2015-10-15 09:07:56 +00:00

Assignee: aklimov
Status: Resolved (closed on 2016-02-17 10:50:03 +00:00)
Target Version: 2.2.0
Last Update: 2016-02-17 10:50:03 +00:00 (in Redmine)


We used to have an insane regex to determine whether output contains HTML. In 39df25f (referring #9036) this got broken by relaxing the check to strip_tags. As a result, most plugin output using something looking like HTML gets wrong treatment and looks ugly. Most prominent example are all kind of usage-hints in case of a misconfigured check command:

Usage:
check_tcp -H host -p port [-w ] [-c ] [-s ]
...

This will be interpreted as HTML right now and shown in an ugly way. I'd at least restore former behaviour, but instead of putting the former regex in place try to find an even better one. Thinking about something like this:

if (preg_match('~<[^>]*["/\'][^>]*>~', $output, $m)) {

That way output is HTML if there is at least something looking like an HTML attribute inside a tag (e.g. , would match also <"> and <'>) or if there is a tag carrying a slash (e.g.

, would match also </>). We could of course be more precise with our regex, but to me this feels as a good compromise between readability, speed and functionality. The following examples should NOT be detected as HTML:
check_ping -H  -w ,%
[-c ]
Error: a < b, c > d

The original intent of the former regex got already broken in 9d2f0be2, so when fixing this one should also try to figure out what that commit was trying to achieve. As we broke this many times before I'd like to see some unit tests in place for the various cases mentioned above.

Cheers,
Thomas

Changesets

2016-02-17 10:49:23 +00:00 by aklimov bd02e5468c7fa0e4f78a5f14ec03ae54acbef16f

PluginOutput helper: use a regex to detect HTML in plugin output

fixes #10366

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2015

Updated by tgelf on 2015-10-15 09:08:12 +00:00

  • Relates set to 9036
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Feb 16, 2016

Updated by elippmann on 2016-02-16 11:03:58 +00:00

  • Category set to Monitoring
  • Target Version set to 2.2.0
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Feb 16, 2016

Updated by elippmann on 2016-02-16 16:00:38 +00:00

  • Status changed from New to Assigned
  • Assigned to set to aklimov
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Feb 17, 2016

Updated by aklimov on 2016-02-17 10:50:04 +00:00

  • Status changed from Assigned to Resolved
  • Done % changed from 0 to 100

Applied in changeset bd02e54.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.