From: Adam T. <aa-...@us...> - 2024-08-09 21:55:30
|
> The parsing is correct: The image argument is a "URI reference" ... The issue here now becomes why the tests were not failing on Linux, I believe. Both Windows and Linux would have had an absolute path (e.g. `.. image:: /home/user/development/docutils/docutils/test/data/circle-broken.svg`). On Windows, this failed due to misinterpreting the drive letter as a scheme. However, because the default `root_prefix` is `/`, when an absolute path is used on Unix, the scheme is `''`, `imagepath.startswith('/')` is `True`, and `root_prefix/imagepath[1:]` just strips and restores the leading slash. I believe that if we are only accepting URI-refereneces, then we should reject absolute paths on UNIX, and the tests should fail. `--image-loading` was added in Docutils 0.18 and `:loading:` was added in 0.21. Local testing shows that on Linux, absolute paths are allowed with `--image-loading embed` or `:loading: embed` in every released version and the current master branch. On Windows, 0.18-0.20 allowed absolute paths, but since 0.21 an error has been raised. The following table demonstrates the matrix. `image_loading` means using `--image-loading embed` and `:loading:` means using `:loading: embed`. My test commands were: ```console $ printf ".. image:: /mnt/s/Development/docutils/docutils/test/data/circle.svg" | rst2html5 --link-stylesheet --image-loading embed $ printf ".. image:: /mnt/s/Development/docutils/docutils/test/data/circle.svg\n :loading: embed" | rst2html5 --link-stylesheet PS> echo ".. image:: S:/Development/docutils/docutils/test/data/circle.svg" | python rst2html5.py --link-stylesheet --image-loading embed PS> echo ".. image:: S:/Development/docutils/docutils/test/data/circle.svg`n :loading: embed" | python rst2html5.py --link-stylesheet ``` *Absolute path support in the image directive with embed mode* | | **Linux** | **Linux** | **Windows** | **Windows** | |------|---------------|-----------|---------------|-------------| | | `image_loading` | `:loading:` | `image_loading` | `:loading:` | | 0.18 | base64 | N/A | base64 | N/A | | 0.19 | base64 | N/A | base64 | N/A | | 0.20 | base64 | N/A | base64 | N/A | | 0.21 | direct | direct | error | error | | 0.22 | direct | direct | error | error | I think we have two options. (1) support absolute paths on every platform, document as such. (2) make absolute paths return an error on Linux. The first restores backwards compatibility, the second is consistent with what we document. > [r9880] A nit-pick here -- this commit leads to URIs like `file://s:/...` on Windows. From Wikipedia's page on file URIs "*`file://path` (i.e. two slashes, without a hostname) is never correct, but is often used.*". I think we should use [Path.as_uri()](https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.as_uri) instead of `.as_posix()`. A --- **[bugs:#493] Test failure on Windows with embedded images** **Status:** open **Created:** Wed Aug 07, 2024 02:25 AM UTC by Adam Turner **Last Updated:** Fri Aug 09, 2024 08:45 AM UTC **Owner:** nobody xref [r9785], [r9853], [r9855] Dear @milde, Thank you for the fix to my recent patch. It seems neither my patch nor the fix addressed the root cause of the test failures, as tests have resumed failing on Windows. I believe the following demonstrates the problem: ```pycon >>> import sys; print(sys.platform) win32 >>> import urllib.parse, urllib.request >>> urllib.request.url2pathname('test/data/circle-broken.svg') 'test\\data\\circle-broken.svg' >>> urllib.parse.unquote('test/data/circle-broken.svg') 'test/data/circle-broken.svg' ``` Currently, we use `imagepath = urllib.request.url2pathname(uri_parts.path)`, which converts path separators to their platform-native format. On UNIX, `url2pathname` simply calls `unquote`, but on Windows it handles UNC paths (``\\host\path\``) and escaped drive letters (``///C|/users/``). I don't know what led to using `url2pathname()`, as it is quite specialised (the docstring notes "not recommended for general use"). Is it possible to use the simpler `unquote()` here? For local file paths (e.g. without a ``file:///`` scheme), should we even be using URI parsing? Perhaps we should use proper path handling if there is no URI scheme (i.e. the user has provided a file-path). A --- Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |