Skip to content

fix(drivers/pikpak): strip subdomain prefix from OSS endpoint for all platforms - #2862

Open
ericwang2006 wants to merge 2 commits into
OpenListTeam:mainfrom
ericwang2006:main
Open

fix(drivers/pikpak): strip subdomain prefix from OSS endpoint for all platforms#2862
ericwang2006 wants to merge 2 commits into
OpenListTeam:mainfrom
ericwang2006:main

Conversation

@ericwang2006

Copy link
Copy Markdown

…ms #2431

Summary

PikPak API returns upload endpoints with a server-specific subdomain prefix
(e.g. vip-lixian-07.upload-a10b.mypikpak.com). These per-node hostnames are
often unresolvable via DNS. The previous code only normalized the endpoint for
the android platform; web and pc platforms used the raw API value,
causing uploads to fail with no such host.

Change: Removed the platform guard so the subdomain prefix is stripped for
all platforms, keeping only the stable base OSS domain.

  • This PR has breaking changes.
  • This PR changes public API, config, storage format, or migration behavior.
  • This PR requires corresponding changes in related repositories.

Testing

  • go test ./... (not run: go.mod requires Go 1.25+, local toolchain too old)
  • Manual test: upload a file to PikPak storage configured with web or pc platform

Checklist

  • I have read CONTRIBUTING.
  • I confirm this contribution follows the repository license, contribution policy, and code of conduct.
  • I have formatted the changed code with gofmt.
  • I have requested review from relevant maintainers or code owners where applicable.

AI Disclosure

  • This PR includes AI-assisted content.

Tools used:

  • OpenCode

Usage scope:

  • Code generation

  • Review assistance

  • I have reviewed and validated all AI-assisted content included in this PR.

  • I can reproduce all AI-assisted content included in this PR without any AI tools.

@xrgzs xrgzs changed the title fix(pikpak): strip subdomain prefix from OSS endpoint for all platfor… fix(drivers/pikpak): strip subdomain prefix from OSS endpoint for all platforms Jul 31, 2026
@xrgzs xrgzs added bug Module: Driver Driver-Related Issue/PR labels Jul 31, 2026

@PIKACHUIM PIKACHUIM left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🙏 感谢贡献

感谢 @ericwang2006 提交此PR!我已完成代码评审,以下是评审结果。


📖 PR背景与需求

PR标题:fix(drivers/pikpak): strip subdomain prefix from OSS endpoint for all platforms

关联Issue#2431

需求说明:修复 PikPak 驱动在 webpc 平台上传文件时出现的 DNS 解析失败问题。PikPak API 返回的上传端点带有服务器特定的子域名前缀(如 vip-lixian-07.upload-a10b.mypikpak.com),这些按节点分配的主机名通常无法通过 DNS 解析。

问题根因:之前的代码仅对 android 平台做了端点规范化处理,webpc 平台直接使用 API 返回的原始值,导致上传失败并报 no such host 错误。

预期目标:移除平台判断逻辑,对所有平台都统一剥离子域名前缀,只保留稳定的基础 OSS 域名,确保所有平台都能成功上传文件。


📋 问题摘要

  • 功能性:修复合理,解决了实际问题
  • 代码质量:代码简洁清晰
  • ⚠️ 改进建议:有2处需要讨论的点

📂 逐文件分析

drivers/pikpak/driver.go

改动意图:修复所有平台的 PikPak 上传端点 DNS 解析问题。

代码修改逻辑

  • 删除:移除了仅针对 android 平台的硬编码端点替换逻辑:

    // 旧代码
    if d.Addition.Platform == "android" {
        params.Endpoint = "mypikpak.net"
    }
  • 新增:添加了通用的子域名前缀剥离逻辑,对所有平台生效:

    // 新代码
    if parts := strings.SplitN(params.Endpoint, ".", 2); len(parts) > 1 {
        params.Endpoint = parts[1]
    }
  • 原理:使用 strings.SplitN(endpoint, ".", 2) 将域名按第一个点分割为最多两部分,取第二部分作为新的端点。例如:

    • vip-lixian-07.upload-a10b.mypikpak.comupload-a10b.mypikpak.com
    • upload-a10b.mypikpak.commypikpak.com(如果只有两级域名)

合理性评估

  • 优点

    • 修复了 webpc 平台的上传失败问题
    • 代码通用性更好,不再依赖平台判断
    • 逻辑清晰,通过分割字符串而非硬编码特定域名
    • 注释详细,解释了为什么要剥离子域名前缀
  • ⚠️ 疑问

    1. 边界情况处理不完整:如果 API 返回的 params.Endpoint 只有一级域名(如 mypikpak.net),分割后 parts[1] 会变成空字符串,导致上传失败。虽然这种情况不太可能,但代码应该处理。

    2. 与注释中的旧代码不一致:注释中提到 endpoint := strings.Join(strings.Split(params.Endpoint, ".")[1:], "."),这是另一种实现方式,会将 vip-lixian-07.upload-a10b.mypikpak.com 变成 upload-a10b.mypikpak.com。但当前代码使用 SplitN(..., 2),对于三级及以上域名处理方式相同,但对于两级域名(如 mypikpak.net)会有不同行为。建议统一。

    3. 硬编码域名的移除是否安全:旧代码对 android 平台硬编码为 mypikpak.net,新代码改为动态剥离前缀。如果 API 返回的基础域名发生变化(如 upload-xyz.mypikpak.comupload-xyz.newdomain.com),新代码能够自适应。但需要确认 PikPak 官方是否保证 API 返回的域名格式稳定。

详细建议

  1. 增强边界情况处理,避免生成空字符串端点:

    // Strip the server-specific subdomain prefix (e.g. "vip-lixian-07.upload-a10b.mypikpak.com" -> "upload-a10b.mypikpak.com")
    // to get the base OSS endpoint that is reliably DNS-resolvable.
    if strings.Contains(params.Endpoint, ".") {
        parts := strings.SplitN(params.Endpoint, ".", 2)
        if len(parts) > 1 && parts[1] != "" {
            params.Endpoint = parts[1]
        }
    }

    这样可以确保:

    • 如果端点只有一级域名(无点),保持不变
    • 如果分割后第二部分为空,保持不变
  2. 或者使用注释中的旧实现方式(更稳健):

    // Strip the server-specific subdomain prefix (e.g. "vip-lixian-07.upload-a10b.mypikpak.com" -> "upload-a10b.mypikpak.com")
    parts := strings.Split(params.Endpoint, ".")
    if len(parts) > 1 {
        params.Endpoint = strings.Join(parts[1:], ".")
    }

    这种方式对于任意多级域名都能正确处理(如 a.b.c.d.eb.c.d.e)。

  3. 添加日志(可选),方便排查问题:

    originalEndpoint := params.Endpoint
    if parts := strings.SplitN(params.Endpoint, ".", 2); len(parts) > 1 && parts[1] != "" {
        params.Endpoint = parts[1]
        utils.Log.Debugf("PikPak: normalized upload endpoint from %s to %s", originalEndpoint, params.Endpoint)
    }

🎯 总体评价

功能性:⭐⭐⭐⭐ - 修复了实际问题,但边界情况处理需要增强
安全性:⭐⭐⭐⭐⭐ - 无安全隐患
代码质量:⭐⭐⭐⭐ - 代码清晰,但边界检查不完整
实现方案:⭐⭐⭐⭐ - 方向正确,实现细节可优化

建议操作

  • ✅ Approve(建议合并)
  • 🔄 Request Changes(需要修改)
  • ❌ Close(建议关闭)

理由:此 PR 修复了一个真实的 bug,方向完全正确。但当前实现存在边界情况处理不完整的问题(如果 params.Endpoint 只有一级域名,会变成空字符串)。建议补充边界检查或使用注释中提到的 strings.Join(parts[1:], ".") 实现方式,确保代码健壮性。修改后可以立即合并。


🔧 推荐修改方案

方案一(最简单,基于当前代码):

// Strip the server-specific subdomain prefix (e.g. "vip-lixian-07.upload-a10b.mypikpak.com" -> "upload-a10b.mypikpak.com")
// to get the base OSS endpoint that is reliably DNS-resolvable.
if parts := strings.SplitN(params.Endpoint, ".", 2); len(parts) > 1 && parts[1] != "" {
    params.Endpoint = parts[1]
}

方案二(更稳健,基于注释中的旧代码):

// Strip the server-specific subdomain prefix (e.g. "vip-lixian-07.upload-a10b.mypikpak.com" -> "upload-a10b.mypikpak.com")
// to get the base OSS endpoint that is reliably DNS-resolvable.
parts := strings.Split(params.Endpoint, ".")
if len(parts) > 1 {
    params.Endpoint = strings.Join(parts[1:], ".")
}

我个人推荐方案二,因为它对任意多级域名都能正确处理,且逻辑与注释中的旧代码一致。


Next Steps / 后续建议

  1. 补充边界情况处理后,可以考虑添加单元测试验证不同格式的端点处理
  2. 如果可能,建议进行实际的上传测试(webpc 平台),确认修复有效

再次感谢你的贡献!👏

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

Labels

bug Module: Driver Driver-Related Issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants