mqtt-tools / mqttwarn

A highly configurable MQTT message router, where the routing targets are notification plugins, primarily written in Python.
https://mqttwarn.readthedocs.io/
Eclipse Public License 2.0
955 stars 183 forks source link

[ntfy] Bug with notification messages longer than 76 characters #672

Closed codebude closed 9 months ago

codebude commented 1 year ago

When sending "longer" messages via ntfy service, the following error arises in the log and the message is not delivered:


mqttwarn  | requests.exceptions.InvalidHeader: Invalid leading whitespace, reserved character(s), or returncharacter(s) in header value: '=?utf-8?q?Das_Ger=C3=A4t_Kaffeevollautomat_vom_Typ_Siemens_TQ703D07_wurde_an?=\n =?utf-8?q?gelernt?='

The problem seems to be the linebreak (\n) in the message. For explanation check this: https://github.com/psf/requests/issues/3488#issuecomment-238605705 (Even if the linked comment talks about leading line breaks, my testings showed that as soon as the line break disappears in the header, the transfer is successful and no error is thrown. Thus I assume the line break is indeed the problem.

In my message definition in mqttwarn.ini there is no linebreak, so I checked the ntfy.py-service code and found this: https://github.com/mqtt-tools/mqttwarn/blob/a8ce8a99869af8d74d0ec724d669497938bdf21f/mqttwarn/services/ntfy.py#LL242C19-L242C19

I think this might be the source of the error, because as per Pythons email.header documentation (https://docs.python.org/3/library/email.header.html) strings longer than 76 chars will be split into multiple lines:

The maximum line length can be specified explicitly via maxlinelen. For splitting the first line to a shorter value (to account for the field header which isn’t included in s, e.g. Subject) pass in the name of the field in header_name. The default maxlinelen is 76, and the default value for header_name is None, meaning it is not taken into account for the first line of a long, split header.

To avoid this behaviour one must explicitly define the maxlinelen parameter. So from my understanding the aforementioned LOC of the ntfy service should look like this (in case you haven't restricted the message length to 76 chars on purpose):


return Header(s=data, charset="utf-8", maxlinelen=10000).encode()

When setting the maxlinelen we maybe should as @binwiederhier what the max message length (in chars) is, that is accepted by ntfy.

amotl commented 1 year ago

Dear Raffael,

thank you for the excellent report, and for including a proper solution. Does the patch GH-674 correspond with yours?

I would love to see this fix in the official image.

When GH-674 will be merged, you will be able to consume the fix by using the nightly tag [^1], if you are actually referring to the OCI images here. You may want to start using it before a new release has been published.

With kind regards, Andreas.

[^1]: Should be available tomorrow morning at 6 o'clock per ef43029c.

codebude commented 1 year ago

Hi Andreas,

Does the patch GH-674 correspond with yours? Yes, that's how I did and it worked perfectly.

Besides from that, I'm not sure if 10000 is technically to large. Most Webservers accept header with length up to either 4k or 8k length. But I don't know how the server behind ntfy is setup. On the other hand I think in practice it might be irrelevant, because I assume nobody will send that long notifications...

amotl commented 11 months ago

mqttwarn 0.35.0 has been released, and should improve the situation on this topic.

amotl commented 9 months ago

Hi,

happy new year again. Like GH-677, I've not tracked this in detail, but also didn't hear about any complaints. Do you think this issue can be considered resolved?

With kind regards, Andreas.

codebude commented 9 months ago

No complaints from my side. It seems to work fine.