refactor: improve VPN DNS target device discovery - #633
Merged
Merged
Conversation
Reviewer's GuideVPN DNS target discovery now uses shared Ip4Config matching with ActiveConnection and device-level fallback passes, supports additional tunnel device types, and centralizes target validation and DNS collection logic. Sequence diagram for shared Ip4Config VPN device matchingsequenceDiagram
participant Handler
participant NetworkManager
participant ActiveConnection
participant Device
Handler->>ActiveConnection: getActiveConnectionIp4Config
ActiveConnection-->>Handler: vpnIp4ConfigPath
Handler->>NetworkManager: activeConnections
loop Pass 1
Handler->>ActiveConnection: getActiveConnectionIp4Config
ActiveConnection-->>Handler: candidateIp4ConfigPath
alt same Ip4Config
Handler->>NetworkManager: findNetworkInterface
NetworkManager-->>Handler: Device
Handler->>Device: tryDeviceTarget
Device-->>Handler: target or no match
end
end
alt no target found
Handler->>NetworkManager: networkInterfaces
loop Pass 2
Handler->>Device: getDeviceIp4Config
Device-->>Handler: deviceIp4ConfigPath
alt same Ip4Config
Handler->>Device: tryDeviceTarget
Device-->>Handler: target or no match
end
end
end
Flow diagram for VPN DNS target discoveryflowchart TD
A[VPN ActiveConnection] --> B[Get VPN Ip4Config]
B --> C[Pass 1: inspect other ActiveConnections]
C --> D{Same Ip4Config?}
D -->|Yes| E[Inspect associated devices]
D -->|No| C
E --> F[tryDeviceTarget]
C --> G{More ActiveConnections?}
G -->|Yes| C
G -->|No| H[Pass 2: inspect all devices]
F --> I{Supported tunnel type and valid target?}
I -->|Yes| J[Apply VPN DNS target]
I -->|No| E
H --> K{Device Ip4Config equals VPN?}
K -->|Yes| F
K -->|No| L{More devices?}
L -->|Yes| H
L -->|No| M[Log no matching VPN virtual device]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="network-service-plugin/src/session/vpndnsmode/vpndnsmodehandler.cpp" line_range="106-130" />
<code_context>
+ // Pass 1：与 VPN AC 共享同一 Ip4Config 的其它 ActiveConnection（tun 系等存在独立 AC 的场景）。
</code_context>
<issue_to_address>
**nitpick:** The existing handler documentation still says that this component discovers a Tun/DNS target, while the implementation now selects Ppp, WireGuard, Generic, and IpTunnel devices as well. Consumers reading the class contract are given an incomplete description of the supported target types.
**Suggested fix:** Update the header comment to describe VPN virtual-device target discovery rather than Tun-only discovery.
</issue_to_address>
1. Extract device matching logic into reusable helper functions (isVpnTunnelDeviceType and tryDeviceTarget) 2. Replace the hardcoded Tun connection type check with a broader Ip4Config-based matching approach 3. Add Pass 1: match any ActiveConnection sharing the same Ip4Config as the VPN (handles tun-like devices with separate ACs) 4. Add Pass 2: device-level fallback matching devices whose own Ip4Config equals the VPN's (handles virtual interfaces without a separate AC, such as ppp/xfrm) 5. Expand supported tunnel device types to include Ppp and WireGuard in addition to Tun, Generic, and IpTunnel 6. Introduce getDeviceIp4Config helper to query the Ip4Config property directly from the device interface The previous implementation only matched connections explicitly typed as Tun, which missed other virtual tunnel types like ppp, wireguard, and xfrm that can also carry VPN DNS. Rewriting the lookup around shared Ip4Config provides a more robust and type-agnostic way to locate the correct device for applying VPN DNS mode. Influence: 1. Verify VPN DNS is applied correctly on tun/tap based VPN connections 2. Test DNS application on ppp-based VPNs (L2TP/PPTP/SSTP) 3. Test DNS application on WireGuard VPN connections 4. Test DNS application on xfrm/ip-tunnel devices 5. Verify behavior when multiple ActiveConnections share the same Ip4Config as the VPN 6. Confirm fallback path works when no ActiveConnection matches but the device-level Ip4Config matches 7. Check that the debug log "[DNS-TRACE] No matching VPN virtual device found" is emitted only when no target is found 8. Verify DNS priority handling (dnsPriority != 0 triggers DNS collection and skips devices with empty DNS) refactor: 改进 VPN DNS 目标设备发现逻辑 1. 将设备匹配逻辑抽取为可复用的辅助函数（isVpnTunnelDeviceType 和 tryDeviceTarget） 2. 用基于 Ip4Config 的更广泛匹配方式替换原先硬编码的 Tun 连接类型判断 3. 新增 Pass 1：匹配与 VPN 共享同一 Ip4Config 的其它 ActiveConnection（处理 tun 系拥有独立 AC 的场景） 4. 新增 Pass 2：设备级兜底，匹配自身 Ip4Config 与 VPN 相同的设备（处理无 独立 AC 的虚拟口，如 ppp/xfrm） 5. 扩展支持的隧道设备类型，在 Tun、Generic、IpTunnel 基础上新增 Ppp 和 WireGuard 6. 新增 getDeviceIp4Config 辅助函数，直接从设备接口查询 Ip4Config 属性 原有实现只能匹配显式声明为 Tun 类型的连接，遗漏了 ppp、wireguard、xfrm 等同样可承载 VPN DNS 的虚拟隧道类型。改为围绕共享 Ip4Config 进行查找，能 够以更健壮、与类型无关的方式定位应用 VPN DNS 模式的正确设备。 Influence: 1. 验证基于 tun/tap 的 VPN 连接中 VPN DNS 能正确应用 2. 测试基于 ppp 的 VPN（L2TP/PPTP/SSTP）的 DNS 应用 3. 测试 WireGuard VPN 连接的 DNS 应用 4. 测试 xfrm/ip-tunnel 设备的 DNS 应用 5. 验证当多个 ActiveConnection 与 VPN 共享同一 Ip4Config 时的行为 6. 确认在无 ActiveConnection 匹配但设备级 Ip4Config 匹配时兜底路径生效 7. 检查调试日志 "[DNS-TRACE] No matching VPN virtual device found" 仅在 未找到目标时输出 8. 验证 DNS 优先级处理（dnsPriority != 0 时触发 DNS 收集并跳过 DNS 为空 的设备） PMS: BUG-377741
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰，边界处理完善。两阶段查找（Pass 1/Pass 2）设计合理，辅助函数 isVpnTunnelDeviceType 和 tryDeviceTarget 的空指针检查、ifindex 校验、DNS 列表空值判断均到位。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 1.可考虑将 getDeviceIp4Config 和 getActiveConnectionIp4Config 合并为一个通用函数，通过传入 D-Bus 接口名参数来消除重复 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： D-Bus 调用在循环中执行，但由于网络连接和设备数量通常较少（<10个），性能影响可接受。如需优化，可考虑批量查询或缓存 Ip4Config 路径。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 未发现安全漏洞。D-Bus 调用使用标准接口，if_nametoindex 输入来自 NetworkManager 而非用户输入，日志仅输出 vpnAc->id() 无敏感信息泄露。 💡 改进建议代码示例// 建议：合并 getActiveConnectionIp4Config 和 getDeviceIp4Config 为通用函数
QString getIp4ConfigPath(const QString &path, const char *interface)
{
QDBusInterface iface(NMService, path, interface, QDBusConnection::systemBus());
return iface.property("Ip4Config").value<QDBusObjectPath>().path();
}
// 调用示例：
// getIp4ConfigPath(acPath, NMConnActiveInterface)
// getIp4ConfigPath(devPath, NMDeviceInterface)本报告由 AI 代码审查工具自动生成 |
caixr23
approved these changes
Sep 21, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 52cyb, caixr23 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous implementation only matched connections explicitly typed as Tun, which missed other virtual tunnel types like ppp, wireguard, and xfrm that can also carry VPN DNS. Rewriting the lookup around shared Ip4Config provides a more robust and type-agnostic way to locate the correct device for applying VPN DNS mode.
Influence:
refactor: 改进 VPN DNS 目标设备发现逻辑
原有实现只能匹配显式声明为 Tun 类型的连接，遗漏了 ppp、wireguard、xfrm
等同样可承载 VPN DNS 的虚拟隧道类型。改为围绕共享 Ip4Config 进行查找，能
够以更健壮、与类型无关的方式定位应用 VPN DNS 模式的正确设备。
Influence:
PMS: BUG-377741
Summary by Sourcery
Make VPN DNS target discovery robust across virtual tunnel device types by matching devices through their shared IPv4 configuration.
Bug Fixes:
Enhancements: