Bug - SMTP mailto:// Notification content-type (HTML/Text) fix and add CI tests (#1660)

.dockerignore

@@ -1,2 +1,18 @@
 .git
 .github
+changedetectionio/processors/__pycache__
+changedetectionio/api/__pycache__
+changedetectionio/model/__pycache__
+changedetectionio/blueprint/price_data_follower/__pycache__
+changedetectionio/blueprint/tags/__pycache__
+changedetectionio/blueprint/__pycache__
+changedetectionio/blueprint/browser_steps/__pycache__
+changedetectionio/fetchers/__pycache__
+changedetectionio/tests/visualselector/__pycache__
+changedetectionio/tests/restock/__pycache__
+changedetectionio/tests/__pycache__
+changedetectionio/tests/fetchers/__pycache__
+changedetectionio/tests/unit/__pycache__
+changedetectionio/tests/proxy_list/__pycache__
+changedetectionio/__pycache__
+

.github/workflows/test-only.yml

@@ -37,6 +37,11 @@ jobs:
           # Build a changedetection.io container and start testing inside
           docker build . -t test-changedetectionio
 
+      - name: Spin up ancillary SMTP+Echo message test server
+        run: |
+          # Debug SMTP server/echo message back server
+          docker run --network changedet-network -d -p 11025:11025 -p 11080:11080  --hostname mailserver test-changedetectionio  bash -c 'python changedetectionio/tests/smtp/smtp-test-server.py' 
+
       - name: Test built container with pytest
         run: |
           
@@ -63,6 +68,11 @@ jobs:
           # restock detection via playwright - added name=changedet here so that playwright/browserless can connect to it
           docker run --rm --name "changedet" -e "FLASK_SERVER_NAME=changedet" -e "PLAYWRIGHT_DRIVER_URL=ws://browserless:3000" --network changedet-network test-changedetectionio  bash -c 'cd changedetectionio;pytest --live-server-port=5004 --live-server-host=0.0.0.0 tests/restock/test_restock.py'
 
+      - name: Test SMTP notification mime types
+        run: |
+          # SMTP content types - needs the 'Debug SMTP server/echo message back server' container from above
+          docker run --rm  --network changedet-network test-changedetectionio bash -c 'cd changedetectionio;pytest tests/smtp/test_notification_smtp.py'
+
       - name: Test with puppeteer fetcher and disk cache
         run: |
           docker run --rm -e "PUPPETEER_DISK_CACHE=/tmp/data/" -e "USE_EXPERIMENTAL_PUPPETEER_FETCH=yes" -e "PLAYWRIGHT_DRIVER_URL=ws://browserless:3000" --network changedet-network test-changedetectionio  bash -c 'cd changedetectionio;pytest tests/fetchers/test_content.py && pytest tests/test_errorhandling.py && pytest tests/visualselector/test_fetch_data.py'

changedetectionio/diff.py

@@ -54,4 +54,5 @@ def render_diff(previous_version_file_contents, newest_version_file_contents, in
 
     # Recursively join lists
     f = lambda L: line_feed_sep.join([f(x) if type(x) is list else x for x in L])
-    return f(rendered_diff)
+    p= f(rendered_diff)
+    return p

changedetectionio/notification.py

@@ -151,9 +151,12 @@ def process_notification(n_object, datastore):
                     # Apprise will default to HTML, so we need to override it
                     # So that whats' generated in n_body is in line with what is going to be sent.
                     # https://github.com/caronc/apprise/issues/633#issuecomment-1191449321
-                    if not 'format=' in url and (n_format == 'text' or n_format == 'markdown'):
+                    if not 'format=' in url and (n_format == 'Text' or n_format == 'Markdown'):
                         prefix = '?' if not '?' in url else '&'
+                        # Apprise format is lowercase text https://github.com/caronc/apprise/issues/633
+                        n_format = n_format.tolower()
                         url = "{}{}format={}".format(url, prefix, n_format)
+                    # If n_format == HTML, then apprise email should default to text/html and we should be sending HTML only
 
                 apobj.add(url)
 

changedetectionio/tests/smtp/smtp-test-server.py

@@ -0,0 +1,42 @@
+#!/usr/bin/python3
+import smtpd
+import asyncore
+
+# Accept a SMTP message and offer a way to retrieve the last message via TCP Socket
+
+last_received_message = b"Nothing"
+
+
+class CustomSMTPServer(smtpd.SMTPServer):
+
+    def process_message(self, peer, mailfrom, rcpttos, data, **kwargs):
+        global last_received_message
+        last_received_message = data
+        print('Receiving message from:', peer)
+        print('Message addressed from:', mailfrom)
+        print('Message addressed to  :', rcpttos)
+        print('Message length        :', len(data))
+        print(data.decode('utf8'))
+        return
+
+
+# Just print out the last message received on plain TCP socket server
+class EchoServer(asyncore.dispatcher):
+
+    def __init__(self, host, port):
+        asyncore.dispatcher.__init__(self)
+        self.create_socket()
+        self.set_reuse_addr()
+        self.bind((host, port))
+        self.listen(5)
+
+    def handle_accepted(self, sock, addr):
+        global last_received_message
+        print('Incoming connection from %s' % repr(addr))
+        sock.send(last_received_message)
+        last_received_message = b''
+
+
+server = CustomSMTPServer(('0.0.0.0', 11025), None)  # SMTP mail goes here
+server2 = EchoServer('0.0.0.0', 11080)  # Echo back last message received
+asyncore.loop()

changedetectionio/tests/smtp/test_notification_smtp.py

@@ -0,0 +1,165 @@
+import json
+import os
+import time
+import re
+from flask import url_for
+from changedetectionio.tests.util import set_original_response, set_modified_response, set_more_modified_response, live_server_setup, \
+    wait_for_all_checks, \
+    set_longer_modified_response
+from changedetectionio.tests.util import extract_UUID_from_client
+import logging
+import base64
+
+# NOTE - RELIES ON mailserver as hostname running, see github build recipes
+smtp_test_server = 'mailserver'
+
+from changedetectionio.notification import (
+    default_notification_body,
+    default_notification_format,
+    default_notification_title,
+    valid_notification_formats,
+)
+
+def test_setup(live_server):
+    live_server_setup(live_server)
+
+def get_last_message_from_smtp_server():
+    import socket
+    global smtp_test_server
+    port = 11080  # socket server port number
+
+    client_socket = socket.socket()  # instantiate
+    client_socket.connect((smtp_test_server, port))  # connect to the server
+
+    data = client_socket.recv(50024).decode()  # receive response
+    client_socket.close()  # close the connection
+    return data
+
+
+# Requires running the test SMTP server
+
+def test_check_notification_email_formats_default_HTML(client, live_server):
+    # live_server_setup(live_server)
+    set_original_response()
+
+    global smtp_test_server
+    notification_url = f'mailto://changedetection@{smtp_test_server}:11025/?to=fff@home.com'
+
+    #####################
+    # Set this up for when we remove the notification from the watch, it should fallback with these details
+    res = client.post(
+        url_for("settings_page"),
+        data={"application-notification_urls": notification_url,
+              "application-notification_title": "fallback-title " + default_notification_title,
+              "application-notification_body": "fallback-body<br> " + default_notification_body,
+              "application-notification_format": 'HTML',
+              "requests-time_between_check-minutes": 180,
+              'application-fetch_backend': "html_requests"},
+        follow_redirects=True
+    )
+    assert b"Settings updated." in res.data
+
+    # Add a watch and trigger a HTTP POST
+    test_url = url_for('test_endpoint', _external=True)
+    res = client.post(
+        url_for("form_quick_watch_add"),
+        data={"url": test_url, "tags": 'nice one'},
+        follow_redirects=True
+    )
+
+    assert b"Watch added" in res.data
+
+    wait_for_all_checks(client)
+    set_longer_modified_response()
+    client.get(url_for("form_watch_checknow"), follow_redirects=True)
+    wait_for_all_checks(client)
+
+    time.sleep(3)
+
+    msg = get_last_message_from_smtp_server()
+    assert len(msg) >= 1
+
+    # The email should have two bodies, and the text/html part should be <br>
+    assert 'Content-Type: text/plain' in msg
+    assert '(added) So let\'s see what happens.\n' in msg  # The plaintext part with \n
+    assert 'Content-Type: text/html' in msg
+    assert '(added) So let\'s see what happens.<br>' in msg  # the html part
+    res = client.get(url_for("form_delete", uuid="all"), follow_redirects=True)
+    assert b'Deleted' in res.data
+
+
+def test_check_notification_email_formats_default_Text_override_HTML(client, live_server):
+    # live_server_setup(live_server)
+
+    # HTML problems? see this
+    # https://github.com/caronc/apprise/issues/633
+
+    set_original_response()
+    global smtp_test_server
+    notification_url = f'mailto://changedetection@{smtp_test_server}:11025/?to=fff@home.com'
+
+    #####################
+    # Set this up for when we remove the notification from the watch, it should fallback with these details
+    res = client.post(
+        url_for("settings_page"),
+        data={"application-notification_urls": notification_url,
+              "application-notification_title": "fallback-title " + default_notification_title,
+              "application-notification_body": default_notification_body,
+              "application-notification_format": 'Text',
+              "requests-time_between_check-minutes": 180,
+              'application-fetch_backend': "html_requests"},
+        follow_redirects=True
+    )
+    assert b"Settings updated." in res.data
+
+    # Add a watch and trigger a HTTP POST
+    test_url = url_for('test_endpoint', _external=True)
+    res = client.post(
+        url_for("form_quick_watch_add"),
+        data={"url": test_url, "tags": 'nice one'},
+        follow_redirects=True
+    )
+
+    assert b"Watch added" in res.data
+
+    wait_for_all_checks(client)
+    set_longer_modified_response()
+    client.get(url_for("form_watch_checknow"), follow_redirects=True)
+    wait_for_all_checks(client)
+
+    time.sleep(3)
+    msg = get_last_message_from_smtp_server()
+    assert len(msg) >= 1
+    #    with open('/tmp/m.txt', 'w') as f:
+    #        f.write(msg)
+
+    # The email should not have two bodies, should be TEXT only
+
+    assert 'Content-Type: text/plain' in msg
+    assert '(added) So let\'s see what happens.\n' in msg  # The plaintext part with \n
+
+    set_original_response()
+    # Now override as HTML format
+    res = client.post(
+        url_for("edit_page", uuid="first"),
+        data={
+            "url": test_url,
+            "notification_format": 'HTML',
+            'fetch_backend': "html_requests"},
+        follow_redirects=True
+    )
+    assert b"Updated watch." in res.data
+    wait_for_all_checks(client)
+
+    time.sleep(3)
+    msg = get_last_message_from_smtp_server()
+    assert len(msg) >= 1
+
+    # The email should have two bodies, and the text/html part should be <br>
+    assert 'Content-Type: text/plain' in msg
+    assert '(removed) So let\'s see what happens.\n' in msg  # The plaintext part with \n
+    assert 'Content-Type: text/html' in msg
+    assert '(removed) So let\'s see what happens.<br>' in msg  # the html part
+
+    res = client.get(url_for("form_delete", uuid="all"), follow_redirects=True)
+    assert b'Deleted' in res.data

changedetectionio/tests/test_notification.py

@@ -3,7 +3,8 @@ import os
 import time
 import re
 from flask import url_for
-from .util import set_original_response, set_modified_response, set_more_modified_response, live_server_setup, wait_for_all_checks
+from .util import set_original_response, set_modified_response, set_more_modified_response, live_server_setup, wait_for_all_checks, \
+    set_longer_modified_response
 from . util import  extract_UUID_from_client
 import logging
 import base64
@@ -272,7 +273,7 @@ def test_notification_validation(client, live_server):
 
 
 def test_notification_custom_endpoint_and_jinja2(client, live_server):
-    time.sleep(1)
+    #live_server_setup(live_server)
 
     # test_endpoint - that sends the contents of a file
     # test_notification_endpoint - that takes a POST and writes it to file (test-datastore/notification.txt)
@@ -283,12 +284,14 @@ def test_notification_custom_endpoint_and_jinja2(client, live_server):
 
     res = client.post(
         url_for("settings_page"),
-        data={"application-notification_title": "New ChangeDetection.io Notification - {{ watch_url }}",
+        data={
+              "application-fetch_backend": "html_requests",
+              "application-minutes_between_check": 180,
               "application-notification_body": '{ "url" : "{{ watch_url }}", "secret": 444 }',
-              # https://github.com/caronc/apprise/wiki/Notify_Custom_JSON#get-parameter-manipulation
+              "application-notification_format": default_notification_format,
               "application-notification_urls": test_notification_url,
-              "application-minutes_between_check": 180,
+              # https://github.com/caronc/apprise/wiki/Notify_Custom_JSON#get-parameter-manipulation
-              "application-fetch_backend": "html_requests"
+              "application-notification_title": "New ChangeDetection.io Notification - {{ watch_url }}",
               },
         follow_redirects=True
     )
@@ -313,7 +316,6 @@ def test_notification_custom_endpoint_and_jinja2(client, live_server):
     client.get(url_for("form_watch_checknow"), follow_redirects=True)
     time.sleep(2)
 
-
     with open("test-datastore/notification.txt", 'r') as f:
         x = f.read()
         j = json.loads(x)
@@ -326,5 +328,9 @@ def test_notification_custom_endpoint_and_jinja2(client, live_server):
         notification_url = f.read()
         assert 'xxx=http' in notification_url
 
-    os.unlink("test-datastore/notification-url.txt")
+    # Should always be automatically detected as JSON content type even when we set it as 'Text' (default)
+    assert os.path.isfile("test-datastore/notification-content-type.txt")
+    with open("test-datastore/notification-content-type.txt", 'r') as f:
+        assert 'application/json' in f.read()
 
+    os.unlink("test-datastore/notification-url.txt")

changedetectionio/tests/util.py

@@ -38,7 +38,25 @@ def set_modified_response():
         f.write(test_return_data)
 
     return None
+def set_longer_modified_response():
+    test_return_data = """<html>
+    <head><title>modified head title</title></head>
+    <body>
+     Some initial text<br>
+     <p>which has this one new line</p>
+     <br>
+     So let's see what happens.  <br>
+     So let's see what happens.  <br>
+      So let's see what happens.  <br>
+     So let's see what happens.  <br>           
+     </body>
+     </html>
+    """
 
+    with open("test-datastore/endpoint-content.txt", "w") as f:
+        f.write(test_return_data)
+
+    return None
 def set_more_modified_response():
     test_return_data = """<html>
     <head><title>modified head title</title></head>
@@ -187,6 +205,10 @@ def live_server_setup(live_server):
         with open("test-datastore/notification-url.txt", "w") as f:
             f.write(request.url)
 
+        if request.content_type:
+            with open("test-datastore/notification-content-type.txt", "w") as f:
+                f.write(request.content_type)
+
         print("\n>> Test notification endpoint was hit.\n", data)
         return "Text was set"
 

changedetectionio/update_worker.py

@@ -32,15 +32,17 @@ class update_worker(threading.Thread):
 
         watch_history = watch.history
         dates = list(watch_history.keys())
+        # Add text that was triggered
+        snapshot_contents = watch.get_history_snapshot(dates[-1])
 
         # HTML needs linebreak, but MarkDown and Text can use a linefeed
         if n_object['notification_format'] == 'HTML':
             line_feed_sep = "<br>"
+            # Snapshot will be plaintext on the disk, convert to some kind of HTML
+            snapshot_contents = snapshot_contents.replace('\n', line_feed_sep)
         else:
             line_feed_sep = "\n"
 
-        # Add text that was triggered
-        snapshot_contents = watch.get_history_snapshot(dates[-1])
         trigger_text = watch.get('trigger_text', [])
         triggered_text = ''
 
@@ -78,6 +80,9 @@ class update_worker(threading.Thread):
         # Would be better if this was some kind of Object where Watch can reference the parent datastore etc
         v = watch.get(var_name)
         if v and not watch.get('notification_muted'):
+            if var_name == 'notification_format' and v == default_notification_format_for_watch:
+                return self.datastore.data['settings']['application'].get('notification_format')
+
             return v
 
         tags = self.datastore.get_all_tags_for_watch(uuid=watch.get('uuid'))