Skip to content

fix(network): filter virtual network interfaces correctly on Windows#723

Merged
lzwind merged 2 commits intolinuxdeepin:masterfrom
re2zero:bugfix
Apr 23, 2026
Merged

fix(network): filter virtual network interfaces correctly on Windows#723
lzwind merged 2 commits intolinuxdeepin:masterfrom
re2zero:bugfix

Conversation

@re2zero
Copy link
Copy Markdown
Contributor

@re2zero re2zero commented Apr 23, 2026

Fix data migration failure when multiple network adapters exist, including virtual adapters like VMware Network Adapter VMnet1. On Windows, virtual adapter names in QNetworkInterface::name() are GUIDs, not "vmnet*", so the previous filter failed to exclude them, causing the app to select the wrong IP address (e.g., 192.168.199.1 instead of the real WLAN IP 192.168.43.204).

Changes:

  • Add isVirtualInterface() helper to check both name() and humanReadableName()
  • Linux: filter virbr*, vmnet*, docker*, veth*, br- prefixes
  • Windows: filter VMware, VirtualBox, Hyper-V, Virtual Ethernet, vEthernet, Bluetooth

This ensures the sender advertises the correct IP address and the web server binds to a reachable network interface, preventing 0B file transfers.

Summary by Sourcery

Filter out virtual and non-routable network interfaces more reliably when selecting the first available IP address.

Bug Fixes:

  • Prevent selection of IP addresses from virtual or bridged adapters (e.g., VMware, VirtualBox, Hyper-V, Docker) when determining the primary IPv4 address.
  • Ensure the application binds its web server to a reachable, physical network interface on both Linux and Windows.

Enhancements:

  • Introduce a reusable helper to detect virtual network interfaces using both technical and human-readable interface names, with improved logging for filtered adapters.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 23, 2026

Reviewer's Guide

Refactors IP selection to centralize virtual network interface filtering into a reusable helper that considers both the technical and human-readable interface names, improving detection of virtual/bridge adapters (including Windows-only cases) so that the application chooses a real, reachable IPv4 address for data transfer and web server binding.

Sequence diagram for getFirstIp interface filtering with isVirtualInterface

sequenceDiagram
    participant CommonUitls
    participant QNetworkInterface
    participant isVirtualInterface

    CommonUitls->>QNetworkInterface: allInterfaces()
    loop for each netInterface
        CommonUitls->>QNetworkInterface: flags()
        QNetworkInterface-->>CommonUitls: interfaceFlags
        alt not up or not running or loopback
            CommonUitls-->>CommonUitls: continue to next interface
        else valid interface flags
            CommonUitls->>isVirtualInterface: isVirtualInterface(netInterface)
            alt is virtual
                isVirtualInterface-->>CommonUitls: true
                CommonUitls-->>CommonUitls: skip virtual interface
            else not virtual
                isVirtualInterface-->>CommonUitls: false
                CommonUitls->>QNetworkInterface: addressEntries()
                QNetworkInterface-->>CommonUitls: entries
                loop for each entry
                    CommonUitls-->>CommonUitls: check IPv4 and not LocalHost
                    alt matches criteria
                        CommonUitls-->>CommonUitls: select IP
                        CommonUitls-->>CommonUitls: log IP and interface name
                        CommonUitls-->>CommonUitls: return IP string
                    end
                end
            end
        end
    end
Loading

Flow diagram for updated getFirstIp virtual interface filtering

flowchart TD
    A[Start getFirstIp] --> B[Initialize empty ip string]
    B --> C[Get list of all network interfaces]
    C --> D[For each netInterface in interfaces]

    D --> E{netInterface.flags contains IsUp
and contains IsRunning
and does not contain IsLoopBack?}
    E -- No --> D
    E -- Yes --> F{isVirtualInterface netInterface?}

    F -- Yes --> D
    F -- No --> G[Get all addressEntries of netInterface]

    G --> H[For each entry in addressEntries]
    H --> I{entry.ip protocol is IPv4
and entry.ip is not LocalHost?}

    I -- No --> H
    I -- Yes --> J[Set ip to entry.ip string]
    J --> K[Log Found available IP with interface name]
    K --> L[Return ip as std::string]

    H -->|No more entries| D
    D -->|No more interfaces| M[Return empty std::string]
    M --> N[End getFirstIp]
Loading

Flow diagram for isVirtualInterface helper logic

flowchart TD
    A[Start isVirtualInterface] --> B[Read netInterface.name and toLower]
    B --> C{name starts with virbr
or vmnet
or docker
or veth
or br-?}
    C -- Yes --> D[Log Filtering virtual interface by name]
    D --> E[Return true]
    C -- No --> F[Read netInterface.humanReadableName and toLower]
    F --> G{humanName contains vmware
or virtualbox
or hyper-v
or virtual ethernet
or vethernet
or bluetooth?}
    G -- Yes --> H[Log Filtering virtual interface by humanReadableName]
    H --> I[Return true]
    G -- No --> J[Return false]
    J --> K[End isVirtualInterface]
Loading

File-Level Changes

Change Details Files
Introduce reusable isVirtualInterface helper to more robustly detect and skip virtual/bridge network interfaces across platforms.
  • Add a static isVirtualInterface(const QNetworkInterface&) helper that lowercases and checks name() for virbr*, vmnet*, docker*, veth*, br- prefixes (primarily Linux/bridge/docker patterns).
  • Extend filtering to also inspect humanReadableName() for substrings like VMware, VirtualBox, Hyper-V, Virtual Ethernet, vEthernet, and Bluetooth to better catch Windows and other virtual/virtualized adapters whose name() is a GUID.
  • Add qInfo() logging when an interface is filtered, distinguishing name-based vs humanReadableName-based filtering for easier diagnostics.
src/common/commonutils.cpp
src/compat/common/commonutils.cpp
Use isVirtualInterface in IP selection to ensure the first advertised IP is from a real, reachable interface and improve logging.
  • Replace inline checks against virbr*/vmnet*/docker* in getFirstIp() with calls to isVirtualInterface() so the same filtering logic is reused in both common and compat builds.
  • Enhance the successful-IP-selection log to include both the chosen IP and the interface name to aid debugging which adapter was selected.
  • Keep the existing behavior of skipping non-running interfaces and localhost while updating the filter criteria to avoid virtual and Bluetooth interfaces that can cause unreachable bindings.
src/common/commonutils.cpp
src/compat/common/commonutils.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • The isVirtualInterface helper is duplicated in two compilation units; consider moving it to a shared header/source to avoid divergence between platform builds over time.
  • The new qInfo() logging inside isVirtualInterface will log on every filtered interface; if this runs frequently, consider lowering the log level or adding a one-time debug summary to reduce log noise.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `isVirtualInterface` helper is duplicated in two compilation units; consider moving it to a shared header/source to avoid divergence between platform builds over time.
- The new `qInfo()` logging inside `isVirtualInterface` will log on every filtered interface; if this runs frequently, consider lowering the log level or adding a one-time debug summary to reduce log noise.

## Individual Comments

### Comment 1
<location path="src/common/commonutils.cpp" line_range="33-41" />
<code_context>
+    if (name.startsWith("virbr") || name.startsWith("vmnet")
+        || name.startsWith("docker") || name.startsWith("veth")
+        || name.startsWith("br-")) {
+        qInfo() << "Filtering virtual interface (by name):" << netInterface.name();
+        return true;
+    }
+
+    QString humanName = netInterface.humanReadableName().toLower();
+    if (humanName.contains("vmware") || humanName.contains("virtualbox")
+        || humanName.contains("hyper-v") || humanName.contains("virtual ethernet")
+        || humanName.contains("vethernet") || humanName.contains("bluetooth")) {
+        qInfo() << "Filtering virtual interface (by humanReadableName):" << netInterface.humanReadableName();
+        return true;
+    }
</code_context>
<issue_to_address>
**suggestion (performance):** Verbose logging for every filtered interface may be noisy in normal operation.

At `qInfo` level this may generate excessive logs on hosts with many virtual/container interfaces, especially if `getFirstIp` runs often. Please consider moving these to a debug-level log or guarding them with a debug/verbose flag to avoid log spam in normal deployments.

Suggested implementation:

```cpp
    if (name.startsWith("virbr") || name.startsWith("vmnet")
        || name.startsWith("docker") || name.startsWith("veth")
        || name.startsWith("br-")) {
        qDebug() << "Filtering virtual interface (by name):" << netInterface.name();
        return true;
    }

```

```cpp
    QString humanName = netInterface.humanReadableName().toLower();
    if (humanName.contains("vmware") || humanName.contains("virtualbox")
        || humanName.contains("hyper-v") || humanName.contains("virtual ethernet")
        || humanName.contains("vethernet") || humanName.contains("bluetooth")) {
        qDebug() << "Filtering virtual interface (by humanReadableName):" << netInterface.humanReadableName();
        return true;
    }

```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/common/commonutils.cpp
@github-actions
Copy link
Copy Markdown

TAG Bot

TAG: 1.2.3
EXISTED: no
DISTRIBUTION: unstable

re2zero added 2 commits April 23, 2026 16:50
Refactor the virtual interface detection logic into a dedicated function and add checks for additional virtual interfaces (e.g., veth, br-, VMware, VirtualBox, Hyper-V, Bluetooth) based on both name and human-readable name. This improves IP address retrieval by more accurately skipping virtual network interfaces.
- bump version to 1.2.3

Log : bump version to 1.2.3
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份 git diff 主要对 dde-cooperation 项目中的网络接口过滤逻辑进行了重构和增强。以下是对代码的详细审查和改进建议:

1. 代码逻辑与功能审查

优点:

  • 功能增强isVirtualInterface 函数的引入显著增强了虚拟接口的识别能力。相比之前仅硬编码检查 virbr, vmnet, docker,新版本增加了对 veth, br- 的识别,并且引入了对 humanReadableName 的检查,能够识别 VMware, VirtualBox, Hyper-V 以及蓝牙适配器。这解决了 changelog 中提到的 "filter virtual network interfaces correctly on Windows" 的问题,因为 Windows 网卡通常通过描述名称(humanReadableName)来区分类型。
  • 代码复用:将过滤逻辑提取为独立的静态函数 isVirtualInterface,消除了代码重复(虽然 diff 中显示在 src/commonsrc/compat 中各复制了一份,但在各自文件内部逻辑更清晰了)。
  • 日志改进:日志输出更加详细,区分了是通过 name 还是 humanReadableName 过滤的,并且在找到 IP 时打印了对应的接口名称,便于调试。

潜在问题与建议:

  • 跨平台兼容性细节

    • 代码中 netInterface.name().toLower() 在 Linux 上通常有效(如 eth0, enp3s0)。但在 Windows 上,接口名称通常是 GUID(如 {xxx-xxx...}),不包含 "vmnet" 等前缀。
    • 分析:代码逻辑已经考虑到了这一点,因为它同时检查了 humanReadableName()。在 Windows 上,GUID 名称不会匹配前缀,但 humanReadableName()(如 "VMware Network Adapter VMnet8")会被匹配并过滤。
    • 建议:目前的逻辑是合理的,但需注意 humanReadableName() 在不同语言环境下的表现。如果系统是中文版 Windows,"VMware" 可能仍然是英文,但 "Hyper-V" 可能会包含中文字符。
      • 当前代码:humanName.contains("hyper-v")
      • 改进建议:如果需要支持非英文系统的完美过滤,可能需要增加对特定厂商 ID(OUI)的检查,或者增加更多语言环境的关键字(但这可能过度设计)。目前的实现对于常见的虚拟化软件通常足够,因为它们驱动描述通常保留英文。
  • 蓝牙接口过滤

    • 代码将 bluetooth 关键字加入黑名单。
    • 建议:这是一个好的实践。蓝牙 PAN (Personal Area Network) 接口通常不适合用于局域网文件传输,过滤掉可以避免选择错误的 IP。

2. 代码质量与可维护性

  • 代码重复
    • isVirtualInterface 函数在 src/common/commonutils.cppsrc/compat/common/commonutils.cpp 中完全相同。
    • 建议:考虑到 src/compat 目录通常用于兼容旧版本或特定平台,如果这两个文件会被同时编译或链接到同一个二进制文件中,建议将此函数提取到一个公共的头文件(如 src/common/commonutils.h 或新建一个 networkutils.h)中实现,以避免维护两份相同的代码。如果它们是针对不同平台构建的(互斥),则现状可以接受。

3. 性能分析

  • 字符串操作
    • 函数中对 namehumanReadableName 都调用了 toLower(),这会分配新的内存。
    • QStringstartsWithcontains 是高效的。
  • 优化建议
    • 由于 getFirstIp() 可能会被频繁调用,且 QNetworkInterface::allInterfaces() 本身是系统调用,开销较大。相比之下,toLower() 的开销相对较小,可以忽略不计。
    • 如果追求极致性能,可以避免 toLower(),使用 Qt 的不区分大小写版本(如 QString::startsWith 接受 Qt::CaseInsensitive 参数),但这会降低代码的可读性(需要写很长的字符串字面量)。目前的写法在性能和可读性之间取得了较好的平衡。

4. 代码安全

  • 输入验证
    • QNetworkInterface 传入是安全的,Qt 内部已处理。
  • 日志安全
    • qInfo() << ... 输出网络接口名称。接口名称通常不包含敏感信息,但在某些极端情况下,如果恶意程序能伪造网卡名称,可能会将恶意代码注入日志。鉴于这是桌面环境应用(dde-cooperation),风险较低。
  • 版权声明更新
    • SPDX-FileCopyrightText 更新为 2023 - 2026 是正确的做法,符合开源协议规范。

5. 具体改进建议代码示例

针对代码重复和可维护性,建议将 isVirtualInterface 声明为内联函数或静态辅助函数放在公共头文件中。如果必须放在 .cpp 中,目前的实现已经足够好。

针对 humanReadableName 的匹配,可以稍微优化一下结构,使其更易扩展:

static bool isVirtualInterface(const QNetworkInterface &netInterface)
{
    // 1. 检查接口名称 (Linux/MacOS 主要依赖此项)
    QString name = netInterface.name().toLower();
    const QStringList virtualPrefixes = {"virbr", "vmnet", "docker", "veth", "br-"};
    for (const QString &prefix : virtualPrefixes) {
        if (name.startsWith(prefix)) {
            qInfo() << "Filtering virtual interface (by name):" << netInterface.name();
            return true;
        }
    }

    // 2. 检查可读名称 (Windows 主要依赖此项)
    QString humanName = netInterface.humanReadableName().toLower();
    // 使用 QStringList 便于管理和扩展关键字
    const QStringList virtualKeywords = {
        "vmware", "virtualbox", "hyper-v", 
        "virtual ethernet", "vethernet", "bluetooth"
    };
    
    for (const QString &keyword : virtualKeywords) {
        if (humanName.contains(keyword)) {
            qInfo() << "Filtering virtual interface (by humanReadableName):" << netInterface.humanReadableName();
            return true;
        }
    }

    return false;
}

改进点说明:

  1. 使用 QStringList 管理关键字,比一连串的 || 运算符更整洁,后续添加新的虚拟化软件支持时只需在列表中添加字符串,无需修改逻辑结构。
  2. 保留了原有的日志逻辑,便于排查问题。
  3. 性能影响极微,循环次数非常少。

总结

这段代码修改质量很高,主要解决了 Windows 平台下虚拟网卡识别不全的问题,并优化了代码结构。没有明显的安全漏洞或严重的性能问题。主要的改进空间在于消除跨文件的代码重复(如果架构允许)以及使用列表结构来管理过滤关键字以提高可维护性。

Copy link
Copy Markdown
Contributor

@pppanghu77 pppanghu77 left a comment

Choose a reason for hiding this comment

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

++

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pppanghu77, re2zero

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lzwind lzwind merged commit b8a6433 into linuxdeepin:master Apr 23, 2026
19 checks passed
@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented Apr 23, 2026

TAG Bot

Tag created successfully

📋 Tag Details
  • Tag Name: 1.2.3
  • Tag SHA: 74ffcbc496d54a88373ee6677756fa74aad2b225
  • Commit SHA: 69e62b3dcaaf615264dee7867442866b5ff8f905
  • Tag Message:
    Release dde-cooperation 1.2.3
    
    
  • Tagger:
    • Name: re2zero
  • Distribution: unstable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants