caronc / apprise

Apprise - Push Notifications that work with just about every platform!
https://hub.docker.com/r/caronc/apprise
BSD 2-Clause "Simplified" License
11.61k stars 406 forks source link

[ntfy] Support for attaching/uploading local files #866

Closed amotl closed 1 year ago

amotl commented 1 year ago

:bulb: The Idea

Dear Chris and Philipp,

over at mqttwarn, we used to use Apprise to submit notifications to ntfy, which worked great. However, because we wanted to have better control within mqttwarn, we are adding a dedicated mqttwarn ntfy adapter now, see mqttwarn ntfy adapter documentation.

However, it would be sweet if the Apprise ntfy adapter could also gain another feature which was important to us, namely to attach/upload local files, so we wanted to leave a corresponding note here.

The general approach we took is, in order to support the corresponding ntfy HTTP API interface, to submit all textual content via HTTP headers, because the HTTP body will be needed to transport the attachment file. This has the drawback that no UTF-8 messages can be submitted, because HTTP headers are only safe for ASCII / Latin-1.

For this reason, and probably for other reasons related to the character of Apprise, support for this feature may not be suitable at all. If you think this is the case, feel free to close the ticket right away.

With kind regards, Andreas.

/cc @caronc, @binwiederhier

References

caronc commented 1 year ago

Is the problem the file attachments, or just the encoding if utf-8 when there is an attachment present (forcing use if http headers)? In the first reference you shared, there was a suggestion that the headers in question (body/title) were encoded. This might reduce the number of characters that a ntfy message can have though.

Another way of doing it would be to leverage yenc or base64 and put the attachments inline. Both create bloat, base64 more-so, but it has more support. Food for thought

binwiederhier commented 1 year ago

This has the drawback that no UTF-8 messages can be submitted, because HTTP headers are only safe for ASCII / Latin-1.

The ntfy API was meant to be simple simple simple, especially when using with curl. Hence the decision to just do "attachments in the body" and the rest in the header. The fact that headers do not officially support UTF-8 (Go does, but I'm not sure other languages do), and obviously are length restricted, is a drawback indeed.

There is a proposal for an extension of the ntfy API to also support multipart messages (https://github.com/binwiederhier/ntfy/issues/69#issuecomment-1183839284, see case 4), but I struggled a bit implementing this so I stopped.

Another option was to encode the headers using RFC 2047, e.g. Message: =?UTF-8?B?VGhyZWUgc2FudGFzIPCfjoXwn46F8J+OhQ==?=. This would be easy to do and I already do this in incoming emails. This would be trivial to implement.

amotl commented 1 year ago

Hi Chris and Philipp,

Is the problem the file attachments, or just the encoding if utf-8 when there is an attachment present (forcing use if http headers)?

a) We needed to use local file attachments, i.e. uploading them using the HTTP body, and the Apprise ntfy adapter does not support that (yet). b) Correct. When using this communication style, and an attachment is present, it will currently not work to use the UTF-8 character set to represent textual notification contents in all the other fields ntfy supports.

The ntfy API was meant to be simple simple simple, especially when using with curl. Hence the decision to just do "attachments in the body" and the rest in the header.

I absolutely appreciated that, it was a pleasure to work with the ntfy HTTP API. Kudos for designing it.

Another option was to encode the headers using RFC 2047, e.g. Message: =?UTF-8?B?VGhyZWUgc2FudGFzIPCfjoXwn46F8J+OhQ==?=. This would be easy to do and I already do this in incoming emails. This would be trivial to implement.

That would be so sweet, indeed! I will be happy to adjust https://github.com/jpmens/mqttwarn/pull/638 accordingly, and, if @caronc will consider this communication style could also be a candidate to make Apprise support ntfy with uploaded attachments, I am sure he will be happy to hear that there are no drawbacks any longer [^1].

With kind regards, Andreas.

[^1]: Other than potential size restrictions on very large notification texts, when web servers are configured too restrictive on the header size detail.

amotl commented 1 year ago

Dear Chris,

thank you for chiming in at https://github.com/jpmens/mqttwarn/pull/638#issuecomment-1518581195, I appreciate it. My intentions have been humble, in order to improve the different client-side implementations for ntfy I've investigated - see also https://github.com/vict0rsch/ntfy-wrapper/issues/8.

Apprise does support local attachments to Ntfy already. I just wanted to chime in should i need to fix a bug of my own. Is this not working anymore?

The attach field is used to attach a file from a URL, which I was referring to as "attaching a remote resource". This is the variant implemented by Apprise, using the HTTP POST method. On the other hand, the attach local file feature of ntfy uses the HTTP PUT method, and is not implemented by Apprise, I think.

Also, in another thread, i saw a discussion about renaming attach to file in the ntfy payload. Is this the case? Or will both keys work for backwards compatibility? Or most likely i just misinterpreted what was being said here 🙂.

I mentioned that detail over at https://github.com/jpmens/mqttwarn/pull/638#discussion_r1172870337, right. However, adding a field called file is exclusively related to introducing a naming convention for mqttwarn, how to submit/signal a reference to a local file, because ntfy does not use a dedicated field for that, but uses the HTTP request body instead.

I hope this can clarify any potential confusion my ramblings may have created. Otherwise, don't hesitate to ask if something is not clear yet.

With kind regards, Andreas.

caronc commented 1 year ago

I believe i can close this ticket off as attachments work fine via Ntfy and Apprise? This was especially improved with #892 .

caronc commented 1 year ago

I think this is done/handled, if not with apprise-api/#118 then definitely with the link above? Closing off unless further discussion is warranted, i'll re-open :rocket: