Skip to content

Conversation

@srikaaviya
Copy link
Contributor

Previously, uploading a binary file (e.g., .pages, .pdf) to replace a Text page caused a 'UnicodeDecodeError or data validation error' because the system attempted to decode the binary content as text based on the existing page type.
This commit:

  1. Checks if the uploaded file is binary based on .
  2. Skips text decoding for binary files.
  3. Updates the item's contenttype metadata to match the uploaded file, ensuring subsequent validation passes.

@UlrichB22
Copy link
Collaborator

@srikaaviya, did you install the pre-commit hooks in your development environment as described in
https://moin-20.readthedocs.io/en/latest/devel/development.html#install-pre-commit-hooks ?

@roland-ruedenauer
Copy link
Contributor

I'm not quite sure if it is a good idea to be able to switch from "text" content to binary content. One immediate problem this will trigger is how diffs between item revisions behave.

Does Moin2 even support switching markup type from e.g. markdown to rST?

@UlrichB22
Copy link
Collaborator

Does Moin2 even support switching markup type from e.g. markdown to rST?

Yes, there is a Convert action in the items view.

@UlrichB22 UlrichB22 marked this pull request as draft January 26, 2026 16:43
@srikaaviya
Copy link
Contributor Author

@srikaaviya, did you install the pre-commit hooks in your development environment as described in https://moin-20.readthedocs.io/en/latest/devel/development.html#install-pre-commit-hooks ?

Yes, I installed and ran the pre-commit hooks.

@srikaaviya
Copy link
Contributor Author

I'm not quite sure if it is a good idea to be able to switch from "text" content to binary content. One immediate problem this will trigger is how diffs between item revisions behave.

Does Moin2 even support switching markup type from e.g. markdown to rST?

Yes, switching content types will cause the Diff view to crash. However, I can implement a safety check to fix this: if the content types are different, the Diff view will simply show a message saying they cannot be compared, instead of error. Will it work?

@UlrichB22
Copy link
Collaborator

Yes, please add the check to avoid tracebacks.
It would be nice if you can squash the first commits.

@srikaaviya srikaaviya force-pushed the fix-binary-upload-error branch from 31e29a4 to 175e5c3 Compare January 29, 2026 04:31
return _crash(item, oldrev, newrev)
try:
diff_html = Markup(item.content._render_data_diff(oldrev, newrev, rev_links=rev_links, fqname=fqname))
except Exception:

Check warning

Code scanning / Bandit

Potential XSS with markupsafe.Markup detected. Do not use Markup on untrusted data. Warning

Potential XSS with markupsafe.Markup detected. Do not use Markup on untrusted data.
@srikaaviya
Copy link
Contributor Author

Yes, please add the check to avoid tracebacks. It would be nice if you can squash the first commits.

Done! I've added the checks to prevent the traceback and squashed the commits.

@UlrichB22
Copy link
Collaborator

Thanks for the updates.

  • IMO the change of the exclusion list for Bandit in pyproject.toml is not related to this topic and should be removed.

  • I successfully tested switching from a moin wiki object to a markdown object. The diff between those revisions works as expected.

  • I also successfully tested switching from a moin wiki object to jpg. The error message on a diff is displayed as plain text and not as warning.

  • The switch back to a text based markup does not work, the resulting object stays as binary. This is ok for me, this is a very rare case.

@srikaaviya srikaaviya force-pushed the fix-binary-upload-error branch from cb9b1c6 to 333115a Compare February 2, 2026 00:24
@srikaaviya
Copy link
Contributor Author

Thanks for review. I reverted the pyproject.toml changes. Also displayed the error message as warning

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

Successfully merging this pull request may close these issues.

3 participants