Skip to content

fix(source): guard NULL curl_buf & fix xy_strcat count in parse_and_say_curl_result - #390

Open
YuruiHong wants to merge 1 commit into
RubyMetric:devfrom
YuruiHong:fix/parse-curl-result-null-guard
Open

fix(source): guard NULL curl_buf & fix xy_strcat count in parse_and_say_curl_result#390
YuruiHong wants to merge 1 commit into
RubyMetric:devfrom
YuruiHong:fix/parse-curl-result-null-guard

Conversation

@YuruiHong

@YuruiHong YuruiHong commented Sep 5, 2026

Copy link
Copy Markdown

问题

curl 未产生任何输出(例如镜像站不可达导致 xy_runpopen 出现异常),或者输出中不含空格(例如 "000")时,parse_and_say_curl_result 里:

char *split = strchr (curl_buf, ' ');   // 可能返回 NULL
if (split) *split = '\0';                // 有防护
...
double speed = xy_str2float (split+1);   // split==NULL 时 split+1 = (char*)0x1

会在 strtof 中解引用非法地址,触发 SIGSEGV

复现

在屏蔽 mirrors.tuna.tsinghua.edu.cn 段 TCP:443 的网络中:

$ sudo chsrc set debian
...
  - 清华大学开源软件镜像站 [精准测速] ... zsh: segmentation fault  sudo chsrc set debian

gdb backtrace(debug 构建)确认 split=NULL 时进入 xy_str2float 后崩溃。

修复

仅一行:split 为 NULL 时不再对 split+1 求值,speed 取 0。

验证

同一网络下 chsrc measure debian 遍历完所有镜像,退出码 0,选出最快源。

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Hi @YuruiHong,

❤️ 感谢你的贡献!你的 PR 当前基于 main 分支,请修改使用 dev 分支

@YuruiHong
YuruiHong force-pushed the fix/parse-curl-result-null-guard branch from fe45b9e to 1766dc9 Compare September 5, 2026 16:54
@YuruiHong
YuruiHong changed the base branch from main to dev September 5, 2026 16:54
@ccmywish ccmywish added this to the v0.2.8 milestone Sep 6, 2026
@ccmywish

ccmywish commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

@YuruiHong 👍 感谢你的调试和解决!


出错原因的另一种可能

或者输出中不含空格(例如 "000")

我们调用 curl 命令的时候用的是:

curl -w "%{http_code} %{speed_download}"

可能 curl 测不到的时候,给 %{http_code} 返回了一个 000,而 %{speed_download} 没有值。但是我们用的 -w 选项后面指定的是一个完整的 format string,这个字符串里包含了一个空格,所以空格是一定存在的,所以你看的 000 其实应该是 000 (后面带一个空格),split+1 此时的确也是超出了范围。

但是你在调试过程中发现 split==NULL,难道 curl 输出的真的是 000?这样的话,我认为是 curl 实现的不够严谨。


维护性

最好分析清楚上述原因,并借此机会加一些注释,调整代码(比如引入新变量,区分 split 前后的两个字符串),增加维护性。


贡献者的身份

可以参照文档 第一次贡献者 注册你的贡献者信息!以及在该文件的 header 处添加你自己为 Contributors (按贡献时间顺序向下排列)

@ccmywish

ccmywish commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

当 curl 未产生任何输出(例如镜像站不可达导致 xy_run 内 popen 出现异常)

另外,我看了一下 xy_run() 的实现, popen 出现异常的时候会在 stderr 中输出 "xy_run_iter_lines(): popen() failed",你调试过程中遇到了吗?

这个时候,curl_buf 整体就为 NULL 了,同样我们没有处理,可以考虑添加代码处理一下。

@YuruiHong

Copy link
Copy Markdown
Author

感谢 review。

我做了两点复核,结论如下:

  1. 您对 curl 输出的判断正确。我用 curl 8.21.0 构造几种必失败场景(TEST-NET IP 192.0.2.1127.0.0.1:1 无监听端口、不可解析域名 .invalid),-w "%{http_code} %{speed_download}" 输出始终000 0 共 5 字节,含空格。

  2. 原始触发本 crash 的 TUNA 阻塞现象我目前无法复现——我所处网络环境已经变化,mirrors.tuna.tsinghua.edu.cn:443 现在能建立连接、能正常返回测速。因此我无法在真机上再次得到当时那份触发 SEGV 的 curl_buf,也就无法客观验证"split == NULL 在实际 curl 调用下真的会发生"。

综合来看,本 PR 剩下的这 1 行改动属于防御性代码,覆盖的是 curl 输出未按 -w 模板产出分隔符的假设情形——我目前拿不到证据说明它是必要的。是否合入请您决定,我不再推动。

@YuruiHong

Copy link
Copy Markdown
Author

感谢 review。

我做了两点复核,结论如下:

  1. 您对 curl 输出的判断正确。我用 curl 8.21.0 构造几种必失败场景(TEST-NET IP 192.0.2.1127.0.0.1:1 无监听端口、不可解析域名 .invalid),-w "%{http_code} %{speed_download}" 输出始终是 000 0 共 5 字节,含空格。

  2. 原始触发本 crash 的 TUNA 阻塞现象我目前无法复现。我所处网络环境已经变化,mirrors.tuna.tsinghua.edu.cn:443 现在能建立连接、能正常返回测速。因此我无法在真机上再次得到当时那份触发 SEGV 的 curl_buf,也就无法客观验证 split == NULL 在实际 curl 调用下真的会发生。

综合来看,本 PR 剩下的这 1 行改动属于防御性代码,覆盖的是 curl 输出未按 -w 模板产出分隔符的假设情形。我目前拿不到证据说明它是必要的。是否合入请您决定,我不再推动。

@ccmywish

ccmywish commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

是的,我这边暂时也无法复现。

另外,出现问题时你使用的版本是多少?是 chsrc v0.2.7 还是 chsrc v0.2.7.1?

如果是 v0.2.7 可能和 #386 #385 有关,也是测速的时候出错(v0.2.7.1 已修复)。但是是在 xy_str2float() 的后几行不远处:

  int  http_code = xy_str2int (curl_buf);
  double   speed = xy_str2float (split+1);
  char *speedstr = to_human_readable_speed (speed);

  if (0==http_code)
    {
      char *msg = ENGLISH ? "ERROR curl output: " : "错误 curl 输出: ";
      println (red (xy_2strcat (msg, curl_buf))); // 原错误代码在这里是 xy_strcat(3, msg, curl_buf);
    }

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.

2 participants