Fix readlink on windows (#10140)

fixes https://github.com/certbot/certbot/issues/10135

i did this by first reverting the bad changes from
https://github.com/certbot/certbot/pull/10077 and then fixing up
comments/documentation

it seems that the code comments
[here](https://github.com/certbot/certbot/blob/v3.0.1/certbot/certbot/compat/filesystem.py#L410-L411)
and in the unit tests that os.readlink always returns the extended form
in python 3.8+ was incorrect

significant credit for this work goes to
https://github.com/certbot/certbot/pull/10136 and
https://github.com/mbs-c for identifying the problem in the code here

certbot/CHANGELOG.md

@@ -18,7 +18,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 * Allow nginx plugin to parse non-breaking spaces in nginx configuration files.
 * Honor --reuse-key when --allow-subset-of-names is set
-*
+* Fixed regression in symlink parsing on Windows that was introduced in Certbot
+  3.1.0.
 
 More details about these changes can be found on our GitHub repo.
 

certbot/certbot/_internal/tests/compat/filesystem_test.py

@@ -631,7 +631,11 @@ class ReadlinkTest(unittest.TestCase):
     @unittest.skipIf(POSIX_MODE, reason='Tests specific to Windows')
     @mock.patch("certbot.compat.filesystem.os.readlink")
     def test_normal_path_windows(self, mock_readlink):
-        # Python >=3.8 (os.readlink always returns the extended form)
+        # test the standard path format
+        mock_readlink.return_value = "C:\\short\\path"
+        assert filesystem.readlink("dummy") == "C:\\short\\path"
+
+        # test the extended form
         mock_readlink.return_value = "\\\\?\\C:\\short\\path"
         assert filesystem.readlink("dummy") == "C:\\short\\path"
 

certbot/certbot/compat/filesystem.py

@@ -389,7 +389,7 @@ def readlink(link_path: str) -> str:
     """
     path = os.readlink(link_path)
 
-    if POSIX_MODE:
+    if POSIX_MODE or not path.startswith('\\\\?\\'):
         return path
 
     # At this point, we know we are on Windows and that the path returned uses

certbot/certbot/compat/os.py

@@ -161,11 +161,11 @@ def fstat(*unused_args, **unused_kwargs):  # type: ignore
                        '(eg. has_min_permissions, has_same_ownership).')
 
 
-# Method os.readlink has a significant behavior change with Python 3.8+. Starting
-# with this version, it will return the resolved path in its "extended-style" form
-# unconditionally, which allows to use more than 259 characters, and its string
-# representation is prepended with "\\?\". Problem is that it does it for any path,
-# and will make equality comparison fail with paths that will use the simple form.
+# On Windows, os.readlink "typically" returns the resolved path in its "extended-style" form which
+# allows use of more than 259 characters and its string representation is prepended with "\\?\". See
+# https://docs.python.org/3/library/os.html#os.readlink. This causes problems for us because the
+# behavior is not consistent (see https://github.com/certbot/certbot/pull/10136) and paths that have
+# this prefix won't match those that do not in simple equality comparisons.
 def readlink(*unused_args, **unused_kwargs):  # type: ignore
     """Method os.readlink() is forbidden"""
     raise RuntimeError('Usage of os.readlink() is forbidden. '