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

Jump to bottom

[dev.icinga.com #6088] Multi-line comments don't work #600

Closed
icinga-migration opened this issue Apr 29, 2014 · 14 comments
Closed

[dev.icinga.com #6088] Multi-line comments don't work #600

icinga-migration opened this issue Apr 29, 2014 · 14 comments
Labels
area/monitoring Affects the monitoring module bug Something isn't working
Milestone
2.0.0-beta2

Comments

@icinga-migration
Copy link

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

Created by gbeutner on 2014-04-29 12:28:25 +00:00

Assignee: jmeyer
Status: Resolved (closed on 2014-12-18 14:41:49 +00:00)
Target Version: 2.0.0-beta2
Last Update: 2014-12-18 15:51:10 +00:00 (in Redmine)

Try submitting a comment with more than one line. Instead of properly escaping the newline Icinga Web sends two commands:

[2014-04-29 14:27:28 +0200]  information/compat: Executing external command: [1398774448] ADD_SVC_COMMENT;atlantis.shroudbox.net;http;1;icingaadmin;Hello
[2014-04-29 14:27:28 +0200]  information/icinga: Creating comment for service atlantis.shroudbox.net!http
[2014-04-29 14:27:28 +0200]  information/compat: Executing external command: World
[2014-04-29 14:27:28 +0200]  warning/compat: External command failed: ../../../lib/icinga/externalcommandprocessor.cpp(76): Throw in function static void icinga::ExternalCommandProcessor::Execute(const icinga::String&)
Dynamic exception type: boost::exception_detail::clone_impl >
std::exception::what: Missing timestamp in command: World
[icinga::StackTrace*] = 
    (0) libicinga.so: void boost::throw_exception >(boost::exception_detail::error_info_injector const&) (+0x1a7) [0x7f02a6b51987] (??:?)
    (1) libicinga.so: void boost::exception_detail::throw_exception_(std::invalid_argument const&, char const*, char const*, int) (+0x69) [0x7f02a6bbd7b9] (??:?)
    (2) libicinga.so: icinga::ExternalCommandProcessor::Execute(icinga::String const&) (+0x595) [0x7f02a6bba355] (??:?)
    (3) libcompat.so: icinga::ExternalCommandListener::CommandPipeThread(icinga::String const&) (+0x249) [0x7f02a565a7c9] (??:?)
    (4) libboost_thread.so.1.54.0:  (+0xba4a) [0x7f02aabdaa4a] (??:?)
    (5) libpthread.so.0:  (+0x8182) [0x7f02abf42182] (pthread_create.c:312 (discriminator 2))
    (6) libc.so.6: clone (+0x6d) [0x7f02ab0e430d] (clone.S:113)


[icinga::ContextTrace*] =

Changesets

2014-08-14 08:23:04 +00:00 by jmeyer dc8181c

Escape linefeeds and carriage returns in commands and show them as html

fixes #6088

2014-12-18 14:41:35 +00:00 by jmeyer 5b1e9be

Make command parameters with multiple lines work, again

fixes #6088
@icinga-migration
Copy link
Author

Updated by elippmann on 2014-04-29 14:26:10 +00:00

I'm afraid we don't support Icinga / Icinga 2 :)

@icinga-migration
Copy link
Author

Updated by tgelf on 2014-04-29 14:57:04 +00:00

Btw: when fixing this please read the docs to find out how to safely handle other special characters like semicolons.

Cheers,
Thomas

@icinga-migration
Copy link
Author

Updated by elippmann on 2014-06-02 07:17:58 +00:00

  • Category set to Monitoring
  • Target Version set to 204

@icinga-migration
Copy link
Author

Updated by jmeyer on 2014-06-11 07:45:33 +00:00

tgelf wrote:

Btw: when fixing this please read the docs to find out how to safely handle other special characters like semicolons.

Cheers,
Thomas

Which documentation? I could not find anything in the icinga(2) docs related to escaping command arguments.

@icinga-migration
Copy link
Author

Updated by jmeyer on 2014-06-30 08:00:50 +00:00

  • Status changed from New to Feedback
  • Assigned to set to tgelf

@icinga-migration
Copy link
Author

Updated by jmeyer on 2014-07-04 14:27:49 +00:00

  • Assigned to changed from tgelf to elippmann

Icinga1.x nor Icinga2 is decoding commands. It's just a naive "read, split based on expected arg count and read until EOL". So we do not have any chance to get multiline comments to work, at least for Icinga1.x. Icinga2 might handle new lines better, but this should probably being kept as it is for the sake of compatibility.

The two possible solutions I can think of are:

  • Change our command forms so that they only provide a single line input for comments (I'd opt for this one)
  • Convert all new lines to spaces before sending the command

The character encoding does not matter as well. Commands are processed 1:1 so I'd suggest to ensure that commands are in utf-8 when sent.

@icinga-migration
Copy link
Author

Updated by tgelf on 2014-07-04 16:00:57 +00:00

Sending UTF-8 is fine. And it is correct that the core doesn't really care. But if someone enters a newline in the frontend this MUST be handled. Otherwise we allow one to hack in commands he is not authorized for. What we have to do at least is protect us against comments like this:

Hello, I'll kill you:
[1404489500] SHUTDOWN_PROGRAM

Legacy frontends care. As far as I know classic does just s/\n/ /. I think we could do even better. What about encoding newlines by transforming them into '\n' - and converting them back by the web interface when showing them? This would allow one to not only safely use but also "see" newlines.

Cheers,
Thomas

@icinga-migration
Copy link
Author

Updated by elippmann on 2014-07-22 13:02:41 +00:00

  • Status changed from Feedback to New
  • Assigned to deleted elippmann
  • Target Version changed from 204 to Backlog

Escaping line feed (and carriage return) to their textual representation is a good idea and we should implement that.

tgelf wrote:

Sending UTF-8 is fine. And it is correct that the core doesn't really care. But if someone enters a newline in the frontend this MUST be handled. Otherwise we allow one to hack in commands he is not authorized for. What we have to do at least is protect us against comments like this:

[...]

Legacy frontends care. As far as I know classic does just s/\n/ /. I think we could do even better. What about encoding newlines by transforming them into '\n' - and converting them back by the web interface when showing them? This would allow one to not only safely use but also "see" newlines.

Cheers,
Thomas

@icinga-migration
Copy link
Author

Updated by jmeyer on 2014-08-14 06:55:43 +00:00

  • Status changed from New to Assigned
  • Assigned to set to jmeyer

@icinga-migration
Copy link
Author

Updated by jmeyer on 2014-08-14 08:23:18 +00:00

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

Applied in changeset dc8181c.

@icinga-migration
Copy link
Author

Updated by ccesario on 2014-12-16 17:32:07 +00:00

Currently I'm getting a similar problem:

[2014-12-16 13:16:48 -0200] information/ExternalCommandListener: Executing external command: [1418743008] ADD_SVC_COMMENT;host1;ping4;1;icingaadmin;linha1
[2014-12-16 13:16:48 -0200] debug/DbEvents: add external command history
[2014-12-16 13:16:48 -0200] notice/ExternalCommandProcessor: Creating comment for service host1!ping4
[2014-12-16 13:16:48 -0200] debug/DbEvents: adding service comment (id = 19) for 'host1!ping4'
[2014-12-16 13:16:48 -0200] debug/DbEvents: adding service comment (id = 19) for 'host1!ping4'
[2014-12-16 13:16:48 -0200] information/ExternalCommandListener: Executing external command: linha2
[2014-12-16 13:16:48 -0200] warning/ExternalCommandListener: External command failed.../../../lib/icinga/externalcommandprocessor.cpp(104): Throw in function static void icinga::ExternalCommandProcessor::Execute(const icinga::String&)
Dynamic exception type: boost::exception_detail::clone_impl >
std::exception::what: Missing timestamp in command: linha2
[icinga::StackTrace*] =
    (0) libicinga.so: void boost::throw_exception >(boost::exception_detail::error_info_injector const&) (+0x112) [0x2b3dd0bec802]
    (1) libicinga.so: void boost::exception_detail::throw_exception_(std::invalid_argument const&, char const*, char const*, int) (+0x69) [0x2b3dd0bec909]
    (2) libicinga.so: icinga::ExternalCommandProcessor::Execute(icinga::String const&) (+0x4b5) [0x2b3dd0bb2a55]
    (3) libcompat.so: icinga::ExternalCommandListener::CommandPipeThread(icinga::String const&) (+0x1fe) [0x2b3ddc75debe]
    (4) libboost_thread.so.1.54.0:  (+0xba4a) [0x2b3db40c1a4a]
    (5) libpthread.so.0:  (+0x8182) [0x2b3db4747182]
    (6) libc.so.6: clone (+0x6d) [0x2b3db5877efd]


[icinga::ContextTrace*] =


[2014-12-16 13:16:48 -0200] information/ExternalCommandListener: Executing external command: linha3
[2014-12-16 13:16:48 -0200] warning/ExternalCommandListener: External command failed.../../../lib/icinga/externalcommandprocessor.cpp(104): Throw in function static void icinga::ExternalCommandProcessor::Execute(const icinga::String&)
Dynamic exception type: boost::exception_detail::clone_impl >
std::exception::what: Missing timestamp in command: linha3
[icinga::StackTrace*] =
    (0) libicinga.so: void boost::throw_exception >(boost::exception_detail::error_info_injector const&) (+0x112) [0x2b3dd0bec802]
    (1) libicinga.so: void boost::exception_detail::throw_exception_(std::invalid_argument const&, char const*, char const*, int) (+0x69) [0x2b3dd0bec909]
    (2) libicinga.so: icinga::ExternalCommandProcessor::Execute(icinga::String const&) (+0x4b5) [0x2b3dd0bb2a55]
    (3) libcompat.so: icinga::ExternalCommandListener::CommandPipeThread(icinga::String const&) (+0x1fe) [0x2b3ddc75debe]
    (4) libboost_thread.so.1.54.0:  (+0xba4a) [0x2b3db40c1a4a]
    (5) libpthread.so.0:  (+0x8182) [0x2b3db4747182]
    (6) libc.so.6: clone (+0x6d) [0x2b3db5877efd]

In the comment text area I put

linha1
linha2
linha3

*icinga2 version: v2.2.0-150-g364f1da
icingaweb2 git version: git 08473ff*

@icinga-migration
Copy link
Author

Updated by jmeyer on 2014-12-18 14:31:06 +00:00

  • Status changed from Resolved to Assigned
  • Target Version changed from Backlog to 2.0.0-beta2
  • Done % changed from 100 to 0

Thanks for reporting this regression, i'll fix it asap :)

@icinga-migration
Copy link
Author

Updated by jmeyer on 2014-12-18 14:41:49 +00:00

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

Applied in changeset 5b1e9be.

@icinga-migration
Copy link
Author

Updated by ccesario on 2014-12-18 15:51:10 +00:00

It's fixed!

Thank you @johannes

@icinga-migration icinga-migration added bug Something isn't working area/monitoring Affects the monitoring module labels Jan 17, 2017
@icinga-migration icinga-migration added this to the 2.0.0-beta2 milestone Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/monitoring Affects the monitoring module bug Something isn't working
Projects
None yet
Milestone
2.0.0-beta2
Development

No branches or pull requests
1 participant
@icinga-migration