Skip to content

[AI-8th] fix: respect virtual host and port in dubbo provider bootstrap #1556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chxbca wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[AI-8th] fix: respect virtual host and port in dubbo provider bootstrap #1556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
614294f
fix: respect virtual host and port in dubbo provider bootstrap
Mar 26, 2026
a3b4fa5
test: cover buildUrls virtual address resolution
Mar 26, 2026
5385579
chore: trigger cla recheck
Mar 26, 2026
386cdf2
fix: avoid protocol config cache collisions for virtual address
Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 41 additions & 15 deletions ...strap-dubbo/src/main/java/com/alipay/sofa/rpc/bootstrap/dubbo/DubboProviderBootstrap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package com.alipay.sofa.rpc.bootstrap.dubbo;

import com.alipay.sofa.rpc.common.annotation.VisibleForTesting;
import org.apache.dubbo.config.ProtocolConfig;
import org.apache.dubbo.config.ServiceConfig;
import com.alipay.sofa.rpc.bootstrap.ProviderBootstrap;
Expand Down Expand Up @@ -97,27 +98,33 @@ private void copyServers(ProviderConfig<T> providerConfig, ServiceConfig service
if (CommonUtils.isNotEmpty(serverConfigs)) {
List<ProtocolConfig> dubboProtocolConfigs = new ArrayList<ProtocolConfig>();
for (ServerConfig serverConfig : serverConfigs) {
// 生成并丢到缓存里
ProtocolConfig protocolConfig = DubboSingleton.SERVER_MAP.get(serverConfig);
if (protocolConfig == null) {
protocolConfig = new ProtocolConfig();
copyServerFields(serverConfig, protocolConfig);
ProtocolConfig old = DubboSingleton.SERVER_MAP.putIfAbsent(serverConfig, protocolConfig);
if (old != null) {
protocolConfig = old;
}
}
ProtocolConfig protocolConfig = getOrCreateProtocolConfig(serverConfig);
dubboProtocolConfigs.add(protocolConfig);
}
serviceConfig.setProtocols(dubboProtocolConfigs);
}
}

private void copyServerFields(ServerConfig serverConfig, ProtocolConfig protocolConfig) {
ProtocolConfig getOrCreateProtocolConfig(ServerConfig serverConfig) {
String serverCacheKey = buildServerCacheKey(serverConfig);
ProtocolConfig protocolConfig = DubboSingleton.SERVER_MAP.get(serverCacheKey);
if (protocolConfig == null) {
protocolConfig = new ProtocolConfig();
copyServerFields(serverConfig, protocolConfig);
ProtocolConfig old = DubboSingleton.SERVER_MAP.putIfAbsent(serverCacheKey, protocolConfig);
if (old != null) {
protocolConfig = old;
}
}
return protocolConfig;
}

@VisibleForTesting
void copyServerFields(ServerConfig serverConfig, ProtocolConfig protocolConfig) {
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyServerFields was changed from private to package-private for test access; the repo commonly annotates such methods with @VisibleForTesting to document intent and discourage production callers from depending on it. Consider adding the annotation (and import) here.

Copilot uses AI. Check for mistakes.
protocolConfig.setId(serverConfig.getId());
protocolConfig.setName(serverConfig.getProtocol());
protocolConfig.setHost(serverConfig.getHost());
protocolConfig.setPort(serverConfig.getPort());
protocolConfig.setHost(resolveHost(serverConfig));
protocolConfig.setPort(resolvePort(serverConfig));
Comment on lines +123 to +127
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyServerFields now derives ProtocolConfig host/port from virtualHost/virtualPort. However, copyServers caches ProtocolConfig instances in DubboSingleton.SERVER_MAP keyed by ServerConfig, and ServerConfig.equals/hashCode only considers protocol/host/port (not virtualHost/virtualPort). This means different ServerConfig instances that share the same bound host/port but differ in virtualHost/virtualPort can collide and reuse a ProtocolConfig with the wrong published address. Consider changing the cache key to include the resolved host/port (or virtualHost/virtualPort), or avoid caching by ServerConfig.

Copilot uses AI. Check for mistakes.
protocolConfig.setAccepts(serverConfig.getAccepts());
protocolConfig.setSerialization(serverConfig.getSerialization());
if (!StringUtils.CONTEXT_SEP.equals(serverConfig.getContextPath())) {
Expand All @@ -132,6 +139,25 @@ private void copyServerFields(ServerConfig serverConfig, ProtocolConfig protocol
protocolConfig.setParameters(serverConfig.getParameters());
}

private String resolveHost(ServerConfig serverConfig) {
return StringUtils.isNotBlank(serverConfig.getVirtualHost()) ? serverConfig.getVirtualHost()
: serverConfig.getHost();
}

private Integer resolvePort(ServerConfig serverConfig) {
return serverConfig.getVirtualPort() != null ? serverConfig.getVirtualPort() : serverConfig.getPort();
}

String buildServerCacheKey(ServerConfig serverConfig) {
StringBuilder sb = new StringBuilder(64);
sb.append(serverConfig.getProtocol()).append(':')
.append(serverConfig.getHost()).append(':')
.append(serverConfig.getPort()).append(':')
.append(resolveHost(serverConfig)).append(':')
.append(resolvePort(serverConfig));
Comment on lines +151 to +157
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildServerCacheKey() concatenates fields with : separators, but hosts (and virtual hosts) can contain : (e.g., IPv6, see NetUtils.toIpString() returning InetAddress.getHostAddress()), which makes the key potentially ambiguous and can cause cache-key collisions between different server configs. Consider using an unambiguous encoding (e.g., length-prefixed segments, escaping, or a small value-object key with proper equals/hashCode, or a delimiter that cannot appear in hosts).

Suggested change
String buildServerCacheKey(ServerConfig serverConfig) {
StringBuilder sb = new StringBuilder(64);
sb.append(serverConfig.getProtocol()).append(':')
.append(serverConfig.getHost()).append(':')
.append(serverConfig.getPort()).append(':')
.append(resolveHost(serverConfig)).append(':')
.append(resolvePort(serverConfig));
/**
* Append a single part into the server cache key in an unambiguous way.
* Uses a simple length-prefixed encoding: &lt;length&gt;#&lt;value&gt;.
*/
private void appendKeyPart(StringBuilder sb, Object part) {
String value = part == null ? "" : String.valueOf(part);
sb.append(value.length()).append('#').append(value);
}
String buildServerCacheKey(ServerConfig serverConfig) {
StringBuilder sb = new StringBuilder(64);
appendKeyPart(sb, serverConfig.getProtocol());
appendKeyPart(sb, serverConfig.getHost());
appendKeyPart(sb, serverConfig.getPort());
appendKeyPart(sb, resolveHost(serverConfig));
appendKeyPart(sb, resolvePort(serverConfig));

Copilot uses AI. Check for mistakes.
return sb.toString();
}

private void copyProvider(ProviderConfig<T> providerConfig, ServiceConfig<T> serviceConfig) {
serviceConfig.setId(providerConfig.getId());
serviceConfig.setInterface(providerConfig.getInterfaceId());
Expand Down Expand Up @@ -194,8 +220,8 @@ public List<String> buildUrls() {
List<String> urls = new ArrayList<String>();
for (ServerConfig server : servers) {
StringBuilder sb = new StringBuilder(200);
sb.append(server.getProtocol()).append("://").append(server.getHost())
.append(":").append(server.getPort()).append(server.getContextPath())
sb.append(server.getProtocol()).append("://").append(resolveHost(server))
.append(":").append(resolvePort(server)).append(server.getContextPath())
.append(providerConfig.getInterfaceId())
.append("?uniqueId=").append(providerConfig.getUniqueId())
.append(getKeyPairs("version", "1.0"))
Expand Down
5 changes: 2 additions & 3 deletions ...rap/bootstrap-dubbo/src/main/java/com/alipay/sofa/rpc/bootstrap/dubbo/DubboSingleton.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.apache.dubbo.config.ProtocolConfig;
import com.alipay.sofa.rpc.base.Destroyable;
import com.alipay.sofa.rpc.config.RegistryConfig;
import com.alipay.sofa.rpc.config.ServerConfig;
import com.alipay.sofa.rpc.context.RpcRuntimeContext;
import org.apache.dubbo.rpc.model.FrameworkModel;

Expand Down Expand Up @@ -48,9 +47,9 @@ public void postDestroy() {
}

/**
* sofa.SeverConfig --> dubbo.ProtocolConfig
* server cache key --> dubbo.ProtocolConfig
*/
final static ConcurrentMap<ServerConfig, ProtocolConfig> SERVER_MAP = new ConcurrentHashMap<>();
final static ConcurrentMap<String, ProtocolConfig> SERVER_MAP = new ConcurrentHashMap<>();

/**
* sofa.RegistryConfig --> dubbo.RegistryConfig
Expand Down
189 changes: 188 additions & 1 deletion ...p-dubbo/src/test/java/com/alipay/sofa/rpc/bootstrap/dubbo/DubboProviderBootstrapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import com.alipay.sofa.rpc.bootstrap.dubbo.demo.DemoServiceImpl;
import com.alipay.sofa.rpc.config.ApplicationConfig;
import com.alipay.sofa.rpc.config.ProviderConfig;
import com.alipay.sofa.rpc.config.ServerConfig;
import org.apache.dubbo.config.ProtocolConfig;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -33,6 +35,7 @@ public class DubboProviderBootstrapTest {

@Before
public void setUp() throws Exception {
DubboSingleton.SERVER_MAP.clear();

ApplicationConfig serverApplacation = new ApplicationConfig();
serverApplacation.setAppName("server");
Expand All @@ -50,4 +53,188 @@ public void setUp() throws Exception {
public void test_dubbo_service_version() {
Assert.assertEquals("1.0.1", dubboProviderBootstrap.getProviderConfig().getParameter("version"));
}
}

/**
* virtualHost and virtualPort should override the bound host and port together.
*/
@Test
public void test_copy_server_fields_use_virtual_host_and_port() throws Exception {
ServerConfig serverConfig = new ServerConfig()
.setProtocol("dubbo")
.setHost("0.0.0.0")
.setPort(12200)
.setVirtualHost("10.0.0.1")
.setVirtualPort(80);
ProtocolConfig protocolConfig = new ProtocolConfig();

dubboProviderBootstrap.copyServerFields(serverConfig, protocolConfig);

Assert.assertEquals("10.0.0.1", protocolConfig.getHost());
Assert.assertEquals(Integer.valueOf(80), protocolConfig.getPort());
}

/**
* Fall back to the original host and port when no virtual address is configured.
*/
@Test
public void test_copy_server_fields_fallback_to_host_and_port() throws Exception {
ServerConfig serverConfig = new ServerConfig()
.setProtocol("dubbo")
.setHost("127.0.0.1")
.setPort(12200);
ProtocolConfig protocolConfig = new ProtocolConfig();

dubboProviderBootstrap.copyServerFields(serverConfig, protocolConfig);

Assert.assertEquals("127.0.0.1", protocolConfig.getHost());
Assert.assertEquals(Integer.valueOf(12200), protocolConfig.getPort());
}

/**
* Blank virtualHost should be treated as unset, while virtualPort still takes effect.
*/
@Test
public void test_copy_server_fields_fallback_to_host_when_virtual_host_is_blank() throws Exception {
ServerConfig serverConfig = new ServerConfig()
.setProtocol("dubbo")
.setHost("127.0.0.1")
.setPort(12200)
.setVirtualHost(" ")
.setVirtualPort(80);
ProtocolConfig protocolConfig = new ProtocolConfig();

dubboProviderBootstrap.copyServerFields(serverConfig, protocolConfig);

Assert.assertEquals("127.0.0.1", protocolConfig.getHost());
Assert.assertEquals(Integer.valueOf(80), protocolConfig.getPort());
}

/**
* If only virtualHost is configured, the original port should still be used.
*/
@Test
public void test_copy_server_fields_use_virtual_host_and_origin_port_when_virtual_port_is_null() throws Exception {
ServerConfig serverConfig = new ServerConfig()
.setProtocol("dubbo")
.setHost("0.0.0.0")
.setPort(12200)
.setVirtualHost("10.0.0.1");
ProtocolConfig protocolConfig = new ProtocolConfig();

dubboProviderBootstrap.copyServerFields(serverConfig, protocolConfig);

Assert.assertEquals("10.0.0.1", protocolConfig.getHost());
Assert.assertEquals(Integer.valueOf(12200), protocolConfig.getPort());
}

/**
* If only virtualPort is configured, the original host should still be used.
*/
@Test
public void test_copy_server_fields_use_origin_host_and_virtual_port_when_virtual_host_is_null() throws Exception {
ServerConfig serverConfig = new ServerConfig()
.setProtocol("dubbo")
.setHost("127.0.0.1")
.setPort(12200)
.setVirtualPort(80);
ProtocolConfig protocolConfig = new ProtocolConfig();

dubboProviderBootstrap.copyServerFields(serverConfig, protocolConfig);

Assert.assertEquals("127.0.0.1", protocolConfig.getHost());
Assert.assertEquals(Integer.valueOf(80), protocolConfig.getPort());
}

/**
* buildUrls should use the resolved virtual address instead of the bound host and port.
*/
@Test
public void test_build_urls_use_virtual_host_and_port() throws Exception {
ServerConfig serverConfig = new ServerConfig()
.setProtocol("dubbo")
.setContextPath("/")
.setHost("0.0.0.0")
.setPort(12200)
.setVirtualHost("10.0.0.1")
.setVirtualPort(80);
dubboProviderBootstrap.getProviderConfig().setServer(serverConfig);
dubboProviderBootstrap.exported = true;

String url = dubboProviderBootstrap.buildUrls().get(0).toString();
Assert.assertTrue(url.startsWith("dubbo://10.0.0.1:80/" + DemoService.class.getName()));
Assert.assertTrue(url.contains("?uniqueId="));
}

/**
* buildUrls should fall back to the bound host and port when no virtual address is configured.
*/
@Test
public void test_build_urls_fallback_to_host_and_port() throws Exception {
ServerConfig serverConfig = new ServerConfig()
.setProtocol("dubbo")
.setContextPath("/")
.setHost("127.0.0.1")
.setPort(12200);
dubboProviderBootstrap.getProviderConfig().setServer(serverConfig);
dubboProviderBootstrap.exported = true;

String url = dubboProviderBootstrap.buildUrls().get(0).toString();
Assert.assertTrue(url.startsWith("dubbo://127.0.0.1:12200/" + DemoService.class.getName()));
Assert.assertTrue(url.contains("?uniqueId="));
}

/**
* Different virtual addresses should not reuse the same cached ProtocolConfig for the same bound address.
*/
@Test
public void test_protocol_config_cache_key_should_distinguish_virtual_address() {
ServerConfig serverConfig1 = new ServerConfig()
.setProtocol("dubbo")
.setHost("0.0.0.0")
.setPort(12200)
.setVirtualHost("10.0.0.1")
.setVirtualPort(80);
ServerConfig serverConfig2 = new ServerConfig()
.setProtocol("dubbo")
.setHost("0.0.0.0")
.setPort(12200)
.setVirtualHost("10.0.0.2")
.setVirtualPort(81);

ProtocolConfig protocolConfig1 = dubboProviderBootstrap.getOrCreateProtocolConfig(serverConfig1);
ProtocolConfig protocolConfig2 = dubboProviderBootstrap.getOrCreateProtocolConfig(serverConfig2);

Assert.assertNotSame(protocolConfig1, protocolConfig2);
Assert.assertEquals("10.0.0.1", protocolConfig1.getHost());
Assert.assertEquals(Integer.valueOf(80), protocolConfig1.getPort());
Assert.assertEquals("10.0.0.2", protocolConfig2.getHost());
Assert.assertEquals(Integer.valueOf(81), protocolConfig2.getPort());
}

/**
* Same resolved virtual address should reuse the cached ProtocolConfig even if different ServerConfig instances
* share the same bound address.
*/
@Test
public void test_protocol_config_cache_key_should_reuse_same_virtual_address() {
ServerConfig serverConfig1 = new ServerConfig()
.setProtocol("dubbo")
.setHost("0.0.0.0")
.setPort(12200)
.setVirtualHost("10.0.0.1")
.setVirtualPort(80);
ServerConfig serverConfig2 = new ServerConfig()
.setProtocol("dubbo")
.setHost("0.0.0.0")
.setPort(12200)
.setVirtualHost("10.0.0.1")
.setVirtualPort(80);

ProtocolConfig protocolConfig1 = dubboProviderBootstrap.getOrCreateProtocolConfig(serverConfig1);
ProtocolConfig protocolConfig2 = dubboProviderBootstrap.getOrCreateProtocolConfig(serverConfig2);

Assert.assertSame(protocolConfig1, protocolConfig2);
Assert.assertEquals("10.0.0.1", protocolConfig1.getHost());
Assert.assertEquals(Integer.valueOf(80), protocolConfig1.getPort());
}
}
Loading