Repository: conan-io/conan
Pull request #16126 (2024-04-22 13:20:02)

fixed wrong servers 200 ok
Changelog: Fix: Improve error message when one URL is not a valid server but still returns 200-ok under a Conan "ping" API call
Docs: Omit

Close https://github.com/conan-io/conan/issues/16104

uilianries: The current change broke the tests, but when running Conan install, the UX is much better now:

```
$ conan --version                                                                                                                                                                                                  
Conan version 2.3.0-dev


$ conan install -r bincrafters --requires=zlib/1.3                                                                                                                                                                 

======== Input profiles ========
Profile host:
[settings]
arch=armv8
build_type=Release
compiler=apple-clang
compiler.cppstd=17
compiler.libcxx=libc++
compiler.version=15
os=Macos

Profile build:
[settings]
arch=armv8
build_type=Release
compiler=apple-clang
compiler.cppstd=17
compiler.libcxx=libc++
compiler.version=15
os=Macos


======== Computing dependency graph ========
zlib/1.3: Not found in local cache, looking in remotes...
zlib/1.3: Checking remote: bincrafters
Graph root
    cli
ERROR: Package 'zlib/1.3' not resolved: Remote https://bincrafters.jfrog.io/artifactory/api/conan/public-conan doesn't seem a valid Conan remote. [Remote: bincrafters].
```

Patch:
diff --git a/conans/client/rest/rest_client_common.py b/conans/client/rest/rest_client_common.py
index b0956c2ce..9a3f76243 100644
--- a/conans/client/rest/rest_client_common.py
+++ b/conans/client/rest/rest_client_common.py
@@ -142,11 +142,14 @@ class RestCommonMethods(object):
             auth = self.auth
         ret = self.requester.get(url, auth=auth, headers=self.custom_headers, verify=self.verify_ssl)
 
-        server_capabilities = ret.headers.get('X-Conan-Server-Capabilities', "")
+        server_capabilities = ret.headers.get('X-Conan-Server-Capabilities')
         if not server_capabilities and not ret.ok:
             # Old Artifactory might return 401/403 without capabilities, we don't want
             # to cache them #5687, so raise the exception and force authentication
             raise get_exception_from_error(ret.status_code)(response_to_str(ret))
+        if server_capabilities is None:
+            # Some servers returning 200-ok, even if not valid repo
+            raise ConanException(f"Remote {self.remote_url} doesn't seem like a valid Conan remote")
 
         return [cap.strip() for cap in server_capabilities.split(",") if cap]
 
diff --git a/conans/util/windows.py b/conans/util/windows.py
index 53cc89c39..89c1a2752 100644
--- a/conans/util/windows.py
+++ b/conans/util/windows.py
@@ -9,8 +9,7 @@ def conan_expand_user(path):
     if path[:1] != '~':
         return path
     # In win these variables should exist and point to user directory, which
-    # must exist. Using context to avoid permanent modification of os.environ
-    old_env = dict(os.environ)
+    # must exist.
     try:
         home = os.environ.get("HOME")
         # Problematic cases of wrong HOME variable
@@ -22,6 +21,6 @@ def conan_expand_user(path):
             del os.environ["HOME"]
         result = os.path.expanduser(path)
     finally:
-        os.environ.clear()
-        os.environ.update(old_env)
+        if home is not None:
+            os.environ["HOME"] = home
     return result


Test patch:
diff --git a/conans/test/integration/command/upload/upload_test.py b/conans/test/integration/command/upload/upload_test.py
index 7a17a61aa..eda0c0695 100644
--- a/conans/test/integration/command/upload/upload_test.py
+++ b/conans/test/integration/command/upload/upload_test.py
@@ -8,6 +8,7 @@ from collections import OrderedDict
 import pytest
 import requests
 from mock import patch
+from requests import Response
 
 from conans.errors import ConanException
 from conans.model.package_ref import PkgReference
@@ -485,6 +486,21 @@ class UploadTest(unittest.TestCase):
         client.run("upload hello0/1.2.1@user/testing -r default")
         assert "Uploading recipe 'hello0/1.2.1@user/testing" in client.out
 
+    def test_server_returns_200_ok(self):
+        # https://github.com/conan-io/conan/issues/16104
+        # If server returns 200 ok, without headers, it raises an error
+        class MyHttpRequester(TestRequester):
+            def get(self, _, **kwargs):
+                resp = Response()
+                resp.status_code = 200
+                return resp
+
+        client = TestClient(requester_class=MyHttpRequester, servers={"default": TestServer()})
+        client.save({"conanfile.py": GenConanfile("hello0", "1.2.1")})
+        client.run("create . ")
+        client.run("upload * -c -r default", assert_error=True)
+        assert "doesn't seem like a valid Conan remote" in client.out
+
 
 def test_upload_only_without_user_channel():
     """
diff --git a/conans/test/integration/remote/multi_remote_test.py b/conans/test/integration/remote/multi_remote_test.py
index 0e40e9a59..515ad32dd 100644
--- a/conans/test/integration/remote/multi_remote_test.py
+++ b/conans/test/integration/remote/multi_remote_test.py
@@ -120,35 +120,37 @@ class MultiRemotesTest(unittest.TestCase):
         client = TestClient(servers=self.servers, inputs=2*["admin", "password"])
 
         self._create(client, "hello0", "0.0")
+        default_remote_rev = client.exported_recipe_revision()
         client.run("install --requires=hello0/0.0@lasote/stable --build missing")
+
         client.run("upload hello0/0.0@lasote/stable -r default")
         sleep(1)  # For timestamp and updates checks
         self._create(client, "hello0", "0.0", modifier=" ")
+        local_remote_rev = client.exported_recipe_revision()
         client.run("install --requires=hello0/0.0@lasote/stable --build missing")
+
         client.run("upload hello0/0.0@lasote/stable#latest -r local")
         client.run("remove '*' -c")
 
         client.run("install --requires=hello0/0.0@lasote/stable")
         # If we don't set a remote we find between all remotes and get the first match
-        self.assertIn("hello0/0.0@lasote/stable#20f1a1ed31481dfb59bdfd22f1aa1093 - Downloaded",
-                      client.out)
+        assert f"hello0/0.0@lasote/stable#{default_remote_rev} - Downloaded" in client.out
         client.run("install --requires=hello0/0.0@lasote/stable --update")
-        self.assertIn("hello0/0.0@lasote/stable#f49dd3f4009e57f521520364d8468757 - Updated",
-                      client.out)
+        assert f"hello0/0.0@lasote/stable#{local_remote_rev} - Updated" in client.out
 
         client.run("install --requires=hello0/0.0@lasote/stable --update -r default")
-        self.assertIn("hello0/0.0@lasote/stable#f49dd3f4009e57f521520364d8468757 - Newer",
+        self.assertIn(f"hello0/0.0@lasote/stable#{local_remote_rev} - Newer",
                       client.out)
 
         sleep(1)  # For timestamp and updates checks
         # Check that it really updates in case of newer package uploaded to the associated remote
         client_b = TestClient(servers=self.servers, inputs=3*["admin", "password"])
         self._create(client_b, "hello0", "0.0", modifier="  ")
+        new_local_remote_rev = client_b.exported_recipe_revision()
         client_b.run("install --requires=hello0/0.0@lasote/stable --build missing")
         client_b.run("upload hello0/0.0@lasote/stable -r local")
         client.run("install --requires=hello0/0.0@lasote/stable --update")
-        self.assertIn("hello0/0.0@lasote/stable#1169dcd60e44cc939d0b475693195bd4 - Updated",
-                      client.out)
+        assert f"hello0/0.0@lasote/stable#{new_local_remote_rev} - Updated" in client.out
 
 
 class MultiRemoteTest(unittest.TestCase):