Fix: Handle binary file uploads on Plain Text pages correctly

contrib/css-build/README.md

@@ -15,4 +15,4 @@ To build `theme.css` from `src/moin/themes/basic/scss/theme.scss` using the SASS
 Bootstrap 5.3 makes use of the deprecated `@import` directive and other deprecated SASS features, which causes many warnings to be output when compiling the Basic theme SCSS file.
 This is a know issue and is currently being worked on (see [dart-sass 1.80.0+ throwing a lot of deprecations](https://github.com/twbs/bootstrap/issues/40962) for details).
 
-The build command in `package.json` uses options to suppress warnings resulting from the use of `@import` and any included dependencies. 
+The build command in `package.json` uses options to suppress warnings resulting from the use of `@import` and any included dependencies.

contrib/wsgi/moin2.wsgi

@@ -24,28 +24,29 @@ import site
 
 moin_dir = os.path.dirname(os.path.abspath(__file__))
 
-if sys.platform == 'win32':
-    site.addsitedir(moin_dir + '-venv-python/Lib/site-packages')
+if sys.platform == "win32":
+    site.addsitedir(moin_dir + "-venv-python/Lib/site-packages")
 else:
-    site.addsitedir(moin_dir + '-venv-{0}/lib/{0}/site-packages'.format(sys.executable))
+    site.addsitedir(moin_dir + "-venv-{0}/lib/{0}/site-packages".format(sys.executable))
 
 # make sure this directory is in sys.path (.lower() avoids duplicate entries on Windows)
 if not (moin_dir in sys.path or moin_dir.lower() in sys.path):
     sys.path.insert(0, moin_dir)
 
 # for debugging sys.path issues, comment out after things are working
-print('== moin2.wsgi sys.path ==')
+print("== moin2.wsgi sys.path ==")
 for p in sys.path:
     print(p)
-print('== end moin2.wsgi sys.path ==')
+print("== end moin2.wsgi sys.path ==")
 
-wiki_config = moin_dir + '/wikiconfig_local.py'
+wiki_config = moin_dir + "/wikiconfig_local.py"
 if not os.path.exists(wiki_config):
-    wiki_config = moin_dir + '/wikiconfig.py'
-print('== wiki_config path =', wiki_config, '==')
+    wiki_config = moin_dir + "/wikiconfig.py"
+print("== wiki_config path =", wiki_config, "==")
 
 # create the Moin (Flask) WSGI application
 from moin.app import create_app
+
 application = create_app(wiki_config)
 
 # please note: if you want to do some wsgi app wrapping, do it like shown below:

requirements.d/development.txt

@@ -7,4 +7,4 @@ lxml
 scrapy>=2.10.1
 pre-commit
 # additional type hints
-types-docutils
+types-docutils

src/moin/apps/frontend/views.py

@@ -2875,35 +2875,53 @@
         revid1, revid2 = revid2, revid1
     common_ct = _common_type(oldrev.meta[CONTENTTYPE], newrev.meta[CONTENTTYPE])
 
-    try:
-        item = Item.create(fqname.fullname, contenttype=common_ct, rev_id=newrev.revid)
-    except AccessDenied:
-        abort(403)
+    if common_ct:
+        try:
+            item = Item.create(fqname.fullname, contenttype=common_ct, rev_id=newrev.revid)
+        except AccessDenied:
+            abort(403)
 
-    # if there are many revisions, create rev_links dict with links to older and newer revisions on diff display
-    rev_links = {}
-    if len(rev_ids) > 2:
-        rev1_idx = rev_ids.index(revid1)
-        rev2_idx = rev_ids.index(revid2)
-        if rev1_idx > 0:
-            rev_links["r1_oldest"] = rev_ids[0]
-            rev_links["r1_older"] = rev_ids[rev1_idx - 1]
-        if rev2_idx > rev1_idx + 1:
-            rev_links["r1_newer"] = rev_ids[rev1_idx + 1]
-        end = len(rev_ids) - 1
-        if rev2_idx < end:
-            rev_links["r2_newer"] = rev_ids[rev2_idx + 1]
-            rev_links["r2_newest"] = rev_ids[-1]
-        if rev2_idx > rev1_idx + 1:
-            rev_links["r2_older"] = rev_ids[rev2_idx - 1]
-    if rev_links:
-        rev_links["revid1"] = revid1
-        rev_links["revid2"] = revid2
+        # if there are many revisions, create rev_links dict with links to older and newer revisions on diff display
+        rev_links = {}
+        if len(rev_ids) > 2:
+            rev1_idx = rev_ids.index(revid1)
+            rev2_idx = rev_ids.index(revid2)
+            if rev1_idx > 0:
+                rev_links["r1_oldest"] = rev_ids[0]
+                rev_links["r1_older"] = rev_ids[rev1_idx - 1]
+            if rev2_idx > rev1_idx + 1:
+                rev_links["r1_newer"] = rev_ids[rev1_idx + 1]
+            end = len(rev_ids) - 1
+            if rev2_idx < end:
+                rev_links["r2_newer"] = rev_ids[rev2_idx + 1]
+                rev_links["r2_newest"] = rev_ids[-1]
+            if rev2_idx > rev1_idx + 1:
+                rev_links["r2_older"] = rev_ids[rev2_idx - 1]
+        if rev_links:
+            rev_links["revid1"] = revid1
+            rev_links["revid2"] = revid2
 
-    try:
-        diff_html = Markup(item.content._render_data_diff(oldrev, newrev, rev_links=rev_links, fqname=fqname))
-    except Exception:
-        return _crash(item, oldrev, newrev)
+        try:
+            diff_html = Markup(item.content._render_data_diff(oldrev, newrev, rev_links=rev_links, fqname=fqname))
+        except Exception:
+            return _crash(item, oldrev, newrev)
+    else:
+        # no common content type (e.g. text/x-moin-wiki vs image/png)
+        # we can't render a diff, so we just show a message.
+        # we use the new revision's item to render the page structure around the message.
+        try:
+            item = Item.create(fqname.fullname, rev_id=newrev.revid)
+        except AccessDenied:
+            abort(403)
+        diff_html = (
+            Markup('<div class="caution">')
+            + _("Cannot generate a comparison view because revisions have different content types.")
+            + Markup("<br>")
+            + _("Old content type: {old_ct}").format(old_ct=oldrev.meta[CONTENTTYPE])
+            + Markup("<br>")
+            + _("New content type: {new_ct}").format(new_ct=newrev.meta[CONTENTTYPE])
+            + Markup("</div>")
+        )
 
     item_may = get_item_permissions(item.fqname, item)
     return render_template("diff.html", item_name=item.name, fqname=item.fqname, diff_html=diff_html, may=item_may)

src/moin/items/__init__.py

@@ -1706,14 +1706,17 @@ def make_previews(item: Item, data: bytes | str | None) -> tuple[list[PreviewDif
 
             form = self.ModifyForm.from_request(request)
             meta, data, contenttype_guessed, comment = form._dump(self)
+            is_text_upload = True
             if data is not None:
 
                 # werkzeug may return form data using type tempfile.SpooledTemporaryFile (issue 1974)
                 if isinstance(data, IOBase) or hasattr(data, "read"):
                     data = data.read()
 
                 # decode text content we may have received in binary form
-                if isinstance(data, bytes) and isinstance(self.content, Text):
+                # but only if it looks like text (or we don't know the type)
+                is_text_upload = not contenttype_guessed or contenttype_guessed.startswith("text/")
+                if is_text_upload and isinstance(data, bytes) and isinstance(self.content, Text):
                     encoding = "utf-8"
                     if contenttype_guessed:
                         if m := re.search("charset=(.+?)($|;)", contenttype_guessed):
@@ -1784,6 +1787,8 @@ def make_previews(item: Item, data: bytes | str | None) -> tuple[list[PreviewDif
 
                     # save the new revision, unlock, delete draft
                     contenttype_qs = request.values.get("contenttype")
+                    if not is_text_upload and contenttype_guessed:
+                        contenttype_qs = contenttype_guessed
                     try:
                         self.modify(meta, data, comment, contenttype_guessed, **{CONTENTTYPE: contenttype_qs})
                     except AccessDenied:

src/moin/utils/registry.py

@@ -5,7 +5,7 @@
 MoinMoin - generic registry base class.
 
 Each registration consists of a factory function together with a list of
-arbitrary arguments. Registered entries can be ordered by priority. 
+arbitrary arguments. Registered entries can be ordered by priority.
 During lookup, each factory is called with the provided arguments and
 may return a callable to indicate a match.
 """