Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML dump: drawio files with wrong URL #1742

Closed
sebix opened this issue Aug 25, 2024 · 7 comments
Closed

HTML dump: drawio files with wrong URL #1742

sebix opened this issue Aug 25, 2024 · 7 comments
Labels
cli good first issue Good for newcomers help wanted we need your help

Comments

@sebix
Copy link
Contributor

sebix commented Aug 25, 2024

With #1741 already applied, we encountered another issue in the HTML dump.

  • Upload a .drawio (text) file as a subitem. Can be as simple as the attached one (Diagram.txt, rename to .drawio first)
  • moin dump-html
  • The page Sandkasten/Diagram.drawio shows a link to +get/+8e1c685bac8c4b2dbbda2f182a09b77b/Sandkasten/Diagram.drawio
  • File not found

The file is actually saved at +get/Sandkasten/Diagram.drawio

The error does not happen for .gpx files, but both file types are text.

@UlrichB22
Copy link
Collaborator

@sebix: Do you have an idea how to simplify the code regarding the different contenttypes?

@sebix
Copy link
Contributor Author

sebix commented Aug 26, 2024

Can you give me a hint which part of the code is relevant for this?

@UlrichB22
Copy link
Collaborator

I think it is the same area that I changed in #1741.

The condition to check whether files are copied as raw item or rendered from markup is based on the contenttype from meta data and additionally based on the file extension. Maybe it is a better idea to render only items with CONTENTTYPE_MARKUP and copy raw files for the other. Regarding CONTENTTYPE_TEXT you should test what the best solution is.

I am uncertain about the file extensions. For security reasons, do we need to check and restrict file extensions based on content type elsewhere, e.g. on item creation? When running dump-html, perhaps we can omit the file extension check?

@sebix
Copy link
Contributor Author

sebix commented Oct 11, 2024

I think it is the same area that I changed in #1741.

With the fix for #1741 applied, the bug still occurs.

@UlrichB22
Copy link
Collaborator

@roland-ruedenauer, I want to test your PR. Can you please help me to reproduce this issue?

Which content type should I choose in the following step?

Upload a .drawio (text) file as a subitem.

I tried with "Binary File" and I can see that the download link in the UI contains the Revision-ID. Don't know why and IMO this is the main problem. Revision-IDs only make sense in case of viewing or downloading an older revision, not the current. After running dump-html with the current unpatched system the link is correct without Revison-ID.

@roland-ruedenauer
Copy link
Contributor

I just uploaded a .drawio file and let moin create the subitm (binary content). Without my change, the link will occur in the download link as part of a href value.

But maybe you're right and the revision-id should not be part of the rendered page content.

In this case, here's what I found out:

The revision contained in the link is introduced by the "everything" converter at

attrib = {xlink.href: Iri(scheme="wiki", authority="", path="/" + item_name, query=f"do=get&rev={rev.revid}")}

Processing of the link is done as part of Converter._expand_document (link_conv) and within the tree traversal Converter.handle_wiki_links will be called and self._get_do_rev(input.query) extracts the revision used to build the final url.

If the "everything" converter would't set the revision (assuming it is CURRENT), it won't be part of the render output.

UlrichB22 added a commit that referenced this issue Mar 12, 2025
Fix dump-html: remove item id of links to binary data (#1742)
@UlrichB22
Copy link
Collaborator

I was able to reproduce the issue when I used the “New Item” button in the “Global index”.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli good first issue Good for newcomers help wanted we need your help
Projects
None yet
Development

No branches or pull requests

3 participants