goproxy icon indicating copy to clipboard operation
goproxy copied to clipboard

fix: query latest before list

Open hunshcn opened this issue 1 year ago • 3 comments

Fix https://github.com/goproxy/goproxy/issues/72

hunshcn avatar Jul 25 '24 15:07 hunshcn

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.87%. Comparing base (0b35852) to head (87cd22b). Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #73   +/-   ##
=======================================
  Coverage   81.87%   81.87%           
=======================================
  Files          11       11           
  Lines        1379     1379           
=======================================
  Hits         1129     1129           
  Misses        243      243           
  Partials        7        7           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 25 '24 16:07 codecov[bot]

ping @aofei

测试覆盖率掉了,但是我也想不出什么 case 去走这个 error

hunshcn avatar Jul 26 '24 16:07 hunshcn

ping @aofei 有空看看吗

hunshcn avatar Aug 11 '24 02:08 hunshcn

ping @aofei

hunshcn avatar Oct 25 '24 19:10 hunshcn

@hunshcn 抱歉过去了这么久(三个月)才回复你🤦‍♂️

我刚看了 #72,如果我没理解错,问题的根本原因在于我们在 GoFetcher.directList 中未明确要求 go list 命令查询 latest 或某个 <version>,导致 go list 可能返回 200 OK 和空 body,从而让 Go modules 操作无法依赖 404/410 进行正常的模块路径推算,对吧?

比如正常情况下模块路径推算应该是:

  1. 请求 /github.com/kubernetes/kubernetes/asdasd/@v/list
  2. 得到 404 Not Found,去掉尾部一级 path element
  3. 请求 /github.com/kubernetes/kubernetes/@v/list
  4. 得到 200 OK 且 body 非空

而我们目前的情况应该是:

  1. 请求 /github.com/kubernetes/kubernetes/asdasd/@v/list
  2. 得到 200 OK 且 body 为空,根据 https://go.dev/ref/mod#goproxy-protocol 里的约定,因为没有内容所以 fall back 到 latest 查询
  3. 请求 /github.com/kubernetes/kubernetes/asdasd/@v/latest
  4. 得到 404 Not Found,去掉尾部一级 path element
  5. 请求 /github.com/kubernetes/kubernetes/@v/list
  6. 得到 200 OK 且 body 非空

所以按照目前的实现,其实也能正常工作,但内部行为并不预期。

我刚看了一下代码,在 Goproxy 改版前其实并不存在这个问题,那时我们的实现方式是这样:

https://github.com/goproxy/goproxy/blob/82ff8e5427c9f4b055e32fb5beda749217d5e541/fetch.go#L209

其中 modAtVer 对应到上面的例子是 github.com/kubernetes/kubernetes/asdasd@latest

所以更准确或性能更优的 fix 方式可能不是去先调用一下 gf.directQuery,而是直接改成 path+"@latest",你觉得呢?

aofei avatar Oct 26 '24 02:10 aofei

老实说我对于 go mod protocol 没有特别深入的了解,但看起来确实是一个不错的方法(从功能上来说)。

hunshcn avatar Oct 26 '24 05:10 hunshcn