search

Icinga / icinga-notifications

Icinga Notifications: new and improved notifications and incident management for Icinga (work in progress, not ready for production yet)
GNU General Public License v2.0
9 stars 0 forks source link

Webhook URLs could use trimming #255

Open martialblog opened 1 month ago

martialblog commented 1 month ago

Had a sneaky little whitespace in the URL, happens sometimes when copying URLs.

Caused this error:

2024-07-25T09:11:33.828Z  ERROR  incident  
Failed to send notification via channel plugin  
{"object": "icinga.vagrant!SomeURL", "incident": "#14", "type": "webhook", "error": "parse \" http://192.168.178.39:8055/b858c733\":
first path segment in URL cannot contain colon"}

I don't know if a trim should happen here or in the Web UI or both.

Also, any what to propagate these kinds of errors to the frontend? So that I can see that my webhook is broken? Maybe even have a button: send test message

julianbrost commented 1 month ago

Not sure what would be the best way here. On one hand, I'd say that if a transformation of the user input happens, it should happen in the web frontend so that the user has a chance to see that changes were done to their input. On the other hand, the form definition is given by the channel plugin so the web frontend doesn't have that much information on the semantics of the config.

@oxzi As you assigned this to yourself, what do you want to do here?

oxzi commented 1 month ago

As I wrote over at https://github.com/Icinga/icinga-notifications-web/issues/257#issuecomment-2252160210, I am against trimming all input fields, as this might be destructive for some inputs. My initial idea was to allow fields to be "sanitizeable", for example by an additional field in ConfigOption. I am against just putting some trim commands in the channel, as this is a bit arbitrary.