Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Shadowsocks-libev be the preferred option; Make wget check certificates; More shellcheck fixes #117

Closed
wants to merge 3 commits into from

Conversation

IceCodeNew
Copy link
Contributor

  1. Make Shadowsocks-libev be the preferred option.
    我仔细检视过提供的 4 个安装选项,除了 libev 版本以外其他的都已经停止更新很久了。Shadowsocks-Go 可以考虑更换成 go-shadowsocks2,但这估计免不了对相关部分的大改——而我并没有使用过 go-shadowsocks2,所以只有看其他人在这方面进行贡献,然后我们可以再讨论究竟应该选择哪一个作为默认的安装选项。
  2. Make wget check certificates.
    我特意在整个 repo 范围内搜寻了一下 --no-check-certificate 或者其他可能的关键词,没有看到有人提 issue 说 wget 检查证书导致了错误,所以才加入这个参数来兼容。
    那么我的意见是没有必要在一开始就加入这个参数,这降低了安全性,很多源代码都用到了 wget 拉下来,有必要通过验证证书确保拉下来的东西是未经篡改过的。
    另外以我自己为例,我环境上 wget 还是一个 alias: wget2 --progress=bar --secure-protocol=PFS --https-enforce=soft,所以我打算一点一点地把脚本中的二进制改为通过绝对路径调用,避免 alias 和 脚本中参数冲突导致问题的可能性。

@IceCodeNew IceCodeNew changed the title Make Shadowsocks-libev be the preferred option; Make wget check certificates. Make Shadowsocks-libev be the preferred option; Make wget check certificates; More shellcheck fixes Jul 1, 2020
我特意在整个 repo 范围内搜寻了一下 `--no-check-certificate` 或者其他可能的关键词,没有看到有人提 issue 说 wget 检查证书导致了错误,所以才加入这个参数来兼容。
那么我的意见是没有必要在一开始就加入这个参数,这降低了安全性,很多源代码都用到了 `wget` 拉下来,有必要通过验证证书确保拉下来的东西是未经篡改过的。
另外以我自己为例,我环境上 `wget` 还是一个 alias: `wget2 --progress=bar --secure-protocol=PFS --https-enforce=soft`,所以我打算一点一点地把脚本中的二进制改为通过绝对路径调用,避免 alias 和 脚本中参数冲突导致问题的可能性。
0. fix SC2068
1. fix SC2196
2. use `[[ … ]]` rather than `[ … ]`
(refer: https://google.github.io/styleguide/shellguide.html#s6.3-tests)
@IceCodeNew
Copy link
Contributor Author

实际跑过一遍 shadowsocks-all.sh 确认没有引入新的问题了。
接下来的计划是加入一项新功能支持:重复运行脚本时支持更新依赖项,并重新编译安装 shadowsocks(这么重要的功能我都很好奇为什么我之前都没有人去做 XDD

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