From 40f0b915129bcb4f1718e9daadeac020f9644912 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@users.noreply.github.com>
Date: Thu, 16 Jan 2025 10:09:44 -0800
Subject: [PATCH] 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                                   |  3 ++-
 .../certbot/_internal/tests/compat/filesystem_test.py  |  6 +++++-
 certbot/certbot/compat/filesystem.py                   |  2 +-
 certbot/certbot/compat/os.py                           | 10 +++++-----
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index 5bb02a3be..8305679a5 100644
--- a/certbot/CHANGELOG.md
+++ b/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.
 
diff --git a/certbot/certbot/_internal/tests/compat/filesystem_test.py b/certbot/certbot/_internal/tests/compat/filesystem_test.py
index 8004282d4..2f5d4e7e5 100644
--- a/certbot/certbot/_internal/tests/compat/filesystem_test.py
+++ b/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"
 
diff --git a/certbot/certbot/compat/filesystem.py b/certbot/certbot/compat/filesystem.py
index 44a5cd68b..22d5d0e5f 100644
--- a/certbot/certbot/compat/filesystem.py
+++ b/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
diff --git a/certbot/certbot/compat/os.py b/certbot/certbot/compat/os.py
index cdfc6fbb1..3aa9a527f 100644
--- a/certbot/certbot/compat/os.py
+++ b/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. '