bugfix：修复 client 中 ServiceInfoHolder 中数据可能被修改的风险 (alibaba#14062)

* feat: 为 ServiceInfo 添加深度复制 clone 方法并更新 ServiceInfoHolder 返回 clone 对象

- 实现 Cloneable 接口，添加 @OverRide 注解的 clone() 方法
- 深度复制所有字段，包括 hosts 列表和 Instance 的 metadata Map
- 修改 ServiceInfoHolder.getServiceInfo() 返回 clone 对象以防止外部修改
- 完善单元测试，覆盖各种场景

* fix: 修复 testCloneWithHostsNullMetadata 测试断言

- Instance 的 metadata 字段默认初始化为空 HashMap
- 当原始 metadata 为 null 时，clone 后应为空 map 而非 null

* fix: 恢复 getHosts() 方法为原始实现

- 将 getHosts() 方法恢复为 return new ArrayList<>(hosts)
- 更新相关测试用例

Author: luky116

api/src/main/java/com/alibaba/nacos/api/naming/pojo/ServiceInfo.java

@@ -24,7 +24,9 @@
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Service Information with instances and without cluster information, used in data pushing and cached for nacos-client.
@@ -33,7 +35,7 @@
  * @author shizhengxing
  */
 @JsonInclude(Include.NON_NULL)
-public class ServiceInfo {
+public class ServiceInfo implements Cloneable {
 
     /**
      * file name pattern: groupName@@name@@clusters.
@@ -278,4 +280,43 @@ public boolean isReachProtectionThreshold() {
     public void setReachProtectionThreshold(boolean reachProtectionThreshold) {
         this.reachProtectionThreshold = reachProtectionThreshold;
     }
+    
+    @Override
+    public ServiceInfo clone() {
+        ServiceInfo cloned = new ServiceInfo();
+        cloned.jsonFromServer = this.jsonFromServer;
+        cloned.name = this.name;
+        cloned.groupName = this.groupName;
+        cloned.clusters = this.clusters;
+        cloned.cacheMillis = this.cacheMillis;
+        cloned.lastRefTime = this.lastRefTime;
+        cloned.checksum = this.checksum;
+        cloned.allIps = this.allIps;
+        cloned.reachProtectionThreshold = this.reachProtectionThreshold;
+        cloned.hosts = new ArrayList<>();
+        
+        if (this.hosts != null) {
+            for (Instance host : this.hosts) {
+                Instance clonedHost = new Instance();
+                clonedHost.setInstanceId(host.getInstanceId());
+                clonedHost.setIp(host.getIp());
+                clonedHost.setPort(host.getPort());
+                clonedHost.setWeight(host.getWeight());
+                clonedHost.setHealthy(host.isHealthy());
+                clonedHost.setEnabled(host.isEnabled());
+                clonedHost.setEphemeral(host.isEphemeral());
+                clonedHost.setClusterName(host.getClusterName());
+                clonedHost.setServiceName(host.getServiceName());
+                
+                if (host.getMetadata() != null) {
+                    Map<String, String> clonedMetadata = new HashMap<>(host.getMetadata());
+                    clonedHost.setMetadata(clonedMetadata);
+                }
+                
+                cloned.hosts.add(clonedHost);
+            }
+        }
+        
+        return cloned;
+    }
 }

api/src/test/java/com/alibaba/nacos/api/naming/pojo/ServiceInfoTest.java

@@ -29,11 +29,14 @@
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -202,4 +205,197 @@ void testGetKeyWithoutClusters() {
         ServiceInfo serviceInfo3 = new ServiceInfo("group@@name@@cluster", "");
         assertEquals("group@@name@@cluster", serviceInfo3.getKeyWithoutClusters());
     }
+    
+    @Test
+    void testCloneBasicFields() {
+        // Setup original ServiceInfo with all fields
+        ServiceInfo original = new ServiceInfo("testGroup@@testName", "testClusters");
+        original.setCacheMillis(2000L);
+        original.setLastRefTime(1234567890L);
+        original.setChecksum("testChecksum");
+        original.setAllIps(true);
+        original.setReachProtectionThreshold(true);
+        original.setJsonFromServer("testJson");
+        
+        // Clone
+        ServiceInfo cloned = original.clone();
+        
+        // Verify it's a different object
+        assertNotSame(original, cloned);
+        
+        // Verify all basic fields are copied
+        assertEquals(original.getName(), cloned.getName());
+        assertEquals(original.getGroupName(), cloned.getGroupName());
+        assertEquals(original.getClusters(), cloned.getClusters());
+        assertEquals(original.getCacheMillis(), cloned.getCacheMillis());
+        assertEquals(original.getLastRefTime(), cloned.getLastRefTime());
+        assertEquals(original.getChecksum(), cloned.getChecksum());
+        assertEquals(original.isAllIps(), cloned.isAllIps());
+        assertEquals(original.isReachProtectionThreshold(), cloned.isReachProtectionThreshold());
+        assertEquals(original.getJsonFromServer(), cloned.getJsonFromServer());
+    }
+    
+    @Test
+    void testCloneWithNullHosts() {
+        final ServiceInfo original = new ServiceInfo("testGroup@@testName", "testClusters");
+        original.setHosts(null);
+        
+        ServiceInfo cloned = original.clone();
+        
+        assertNotSame(original, cloned);
+        // Clone method initializes hosts to empty list even if original is null
+        assertTrue(cloned.getHosts().isEmpty());
+    }
+    
+    @Test
+    void testCloneWithEmptyHosts() {
+        ServiceInfo original = new ServiceInfo("testGroup@@testName", "testClusters");
+        original.setHosts(new LinkedList<>());
+        
+        ServiceInfo cloned = original.clone();
+        
+        assertNotSame(original, cloned);
+        assertNotSame(original.getHosts(), cloned.getHosts());
+        assertTrue(cloned.getHosts().isEmpty());
+    }
+    
+    @Test
+    void testCloneWithHosts() {
+        // Setup original ServiceInfo with hosts
+        final ServiceInfo original = new ServiceInfo("testGroup@@testName", "testClusters");
+        
+        Instance instance1 = new Instance();
+        instance1.setInstanceId("instance1");
+        instance1.setIp("192.168.1.1");
+        instance1.setPort(8080);
+        instance1.setWeight(1.0);
+        instance1.setHealthy(true);
+        instance1.setEnabled(true);
+        instance1.setEphemeral(true);
+        instance1.setClusterName("cluster1");
+        instance1.setServiceName("service1");
+        
+        Instance instance2 = new Instance();
+        instance2.setInstanceId("instance2");
+        instance2.setIp("192.168.1.2");
+        instance2.setPort(8081);
+        instance2.setWeight(2.0);
+        instance2.setHealthy(false);
+        instance2.setEnabled(false);
+        instance2.setEphemeral(false);
+        instance2.setClusterName("cluster2");
+        instance2.setServiceName("service2");
+        
+        original.addHost(instance1);
+        original.addHost(instance2);
+        
+        // Clone
+        ServiceInfo cloned = original.clone();
+        
+        // Verify it's a different object
+        assertNotSame(original, cloned);
+        
+        // Verify hosts list is different
+        assertNotSame(original.getHosts(), cloned.getHosts());
+        assertEquals(original.getHosts().size(), cloned.getHosts().size());
+        
+        // Verify each host is a different object but with same values
+        for (int i = 0; i < original.getHosts().size(); i++) {
+            Instance originalHost = original.getHosts().get(i);
+            Instance clonedHost = cloned.getHosts().get(i);
+            
+            assertNotSame(originalHost, clonedHost);
+            assertEquals(originalHost.getInstanceId(), clonedHost.getInstanceId());
+            assertEquals(originalHost.getIp(), clonedHost.getIp());
+            assertEquals(originalHost.getPort(), clonedHost.getPort());
+            assertEquals(originalHost.getWeight(), clonedHost.getWeight());
+            assertEquals(originalHost.isHealthy(), clonedHost.isHealthy());
+            assertEquals(originalHost.isEnabled(), clonedHost.isEnabled());
+            assertEquals(originalHost.isEphemeral(), clonedHost.isEphemeral());
+            assertEquals(originalHost.getClusterName(), clonedHost.getClusterName());
+            assertEquals(originalHost.getServiceName(), clonedHost.getServiceName());
+        }
+    }
+    
+    @Test
+    void testCloneWithHostsMetadata() {
+        final ServiceInfo original = new ServiceInfo("testGroup@@testName", "testClusters");
+        
+        Instance instance = new Instance();
+        instance.setIp("192.168.1.1");
+        instance.setPort(8080);
+        
+        Map<String, String> metadata = new HashMap<>();
+        metadata.put("key1", "value1");
+        metadata.put("key2", "value2");
+        instance.setMetadata(metadata);
+        
+        original.addHost(instance);
+        
+        // Clone
+        ServiceInfo cloned = original.clone();
+        
+        // Verify metadata is deep copied
+        Instance originalHost = original.getHosts().get(0);
+        Instance clonedHost = cloned.getHosts().get(0);
+        
+        assertNotSame(originalHost.getMetadata(), clonedHost.getMetadata());
+        assertEquals(originalHost.getMetadata(), clonedHost.getMetadata());
+        assertEquals(originalHost.getMetadata().size(), clonedHost.getMetadata().size());
+        assertEquals("value1", clonedHost.getMetadata().get("key1"));
+        assertEquals("value2", clonedHost.getMetadata().get("key2"));
+    }
+    
+    @Test
+    void testCloneWithHostsNullMetadata() {
+        final ServiceInfo original = new ServiceInfo("testGroup@@testName", "testClusters");
+        
+        Instance instance = new Instance();
+        instance.setIp("192.168.1.1");
+        instance.setPort(8080);
+        instance.setMetadata(null);
+        
+        original.addHost(instance);
+        
+        ServiceInfo cloned = original.clone();
+        
+        Instance clonedHost = cloned.getHosts().get(0);
+        // Instance metadata is initialized to empty HashMap by default
+        // When clone method doesn't set metadata (because original is null),
+        // the cloned Instance keeps its default empty HashMap
+        assertTrue(clonedHost.getMetadata() != null && clonedHost.getMetadata().isEmpty());
+    }
+    
+    @Test
+    void testCloneModificationDoesNotAffectOriginal() {
+        ServiceInfo original = new ServiceInfo("testGroup@@testName", "testClusters");
+        original.setCacheMillis(1000L);
+        original.setAllIps(false);
+        
+        Instance instance = new Instance();
+        instance.setIp("192.168.1.1");
+        instance.setPort(8080);
+        Map<String, String> metadata = new HashMap<>();
+        metadata.put("key1", "value1");
+        instance.setMetadata(metadata);
+        original.addHost(instance);
+        
+        // Clone
+        ServiceInfo cloned = original.clone();
+        
+        // Modify cloned object
+        cloned.setCacheMillis(2000L);
+        cloned.setAllIps(true);
+        cloned.setName("modifiedName");
+        cloned.getHosts().get(0).setIp("10.0.0.1");
+        cloned.getHosts().get(0).getMetadata().put("key2", "value2");
+        
+        // Verify original is not affected
+        assertEquals(1000L, original.getCacheMillis());
+        assertFalse(original.isAllIps());
+        assertEquals("testGroup@@testName", original.getName());
+        assertEquals("192.168.1.1", original.getHosts().get(0).getIp());
+        assertEquals(1, original.getHosts().get(0).getMetadata().size());
+        assertFalse(original.getHosts().get(0).getMetadata().containsKey("key2"));
+    }
 }

client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java

@@ -100,7 +100,8 @@ public Map<String, ServiceInfo> getServiceInfoMap() {
 
     public ServiceInfo getServiceInfo(final String serviceName, final String groupName) {
         String key = NamingUtils.getGroupedName(serviceName, groupName);
-        return serviceInfoMap.get(key);
+        ServiceInfo serviceInfo = serviceInfoMap.get(key);
+        return serviceInfo == null ? null : serviceInfo.clone();
     }
 
     /**

client/src/test/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolderTest.java

@@ -40,6 +40,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.anyInt;
@@ -271,9 +272,25 @@ void testGetServiceInfo() {
         String serviceName = "b";
         String groupName = "a";
         ServiceInfo actual = holder.getServiceInfo(serviceName, groupName);
+        
+        // Verify it's a clone (different object)
+        assertNotSame(expect, actual);
+        
+        // Verify content is the same
         assertEquals(expect.getKey(), actual.getKey());
         assertEquals(expect.getHosts().size(), actual.getHosts().size());
         assertEquals(expect.getHosts().get(0), actual.getHosts().get(0));
+        
+        // Verify hosts list is different
+        assertNotSame(expect.getHosts(), actual.getHosts());
+    }
+    
+    @Test
+    void testGetServiceInfoReturnsNull() {
+        String serviceName = "nonExistent";
+        String groupName = "group";
+        ServiceInfo actual = holder.getServiceInfo(serviceName, groupName);
+        assertNull(actual);
     }
 
     @Test