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

Webhook Channel Configuration could be stricter and use trim #257

Open
martialblog opened this issue Jul 25, 2024 · 1 comment
Open

Webhook Channel Configuration could be stricter and use trim #257

martialblog opened this issue Jul 25, 2024 · 1 comment

Comments

@martialblog
Copy link
Member

Hi,

the Webhook Channel Configuration HTTP Method and Response Status Codes could be stricter in what input they accept.

Response Status Codes are likely always [0-9]+ and HTTP Method an enum (get, post, etc...)

2024-07-25T10:08:34.214Z ERROR channel Failed to set channel plugin config, terminating the plugin {"id": 2, "name": "Captain Hook", "error": "failed to set plugin config: cannot convert status code \"Lorem ipsum dolor sit amet\" to int: strconv.Atoi: parsing \"Lorem ipsum dolor sit amet\": invalid syntax"}

Also the values could use a trim before being used, not sure if the backend or frontend should do that:

2024-07-25T10:09:56.207Z ERROR channel Failed to set channel plugin config, terminating the plugin {"id": 2, "name": "Captain Hook", "error": "failed to set plugin config: cannot convert status code \" 200 \" to int: strconv.Atoi: parsing \" 200 \": invalid syntax"}

Those sneaky whitespaces

@martialblog martialblog changed the title Webhook Channel Configuration could be stricter use trim Webhook Channel Configuration could be stricter and use trim Jul 25, 2024
@oxzi
Copy link
Member

oxzi commented Jul 26, 2024

This might be related to your other issues Icinga/icinga-notifications#255 and Icinga/icinga-notifications#258.

I would advise against trimming strings in web, as there might be the obscure case for obscure channels where either leading or trailing spaces are necessary. Thus, I would prefer to address Icinga/icinga-notifications#255 on the daemon side.

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

No branches or pull requests

2 participants