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

fix: no more Restricted Shell #772

Merged
merged 1 commit into from
Jul 19, 2023
Merged

fix: no more Restricted Shell #772

merged 1 commit into from
Jul 19, 2023

Conversation

WaterLemons2k
Copy link
Contributor

What does this PR do?

No longer running commands with Restricted Shell, as some Shells don't support it.

Determining whether a Restricted Shell is supported may add unnecessary complexity.

Fixes #771.

Motivation

#771

Additional Notes

The following code can be used to fallback when a Restricted Shell is not supported:

// run cmd
out, err := execCmd.CombinedOutput()
str := string(out)
if err != nil && strings.Contains(str, "bash: illegal option -r") {
	// 如果不支持受限 Shell 则使用 `bash -c` 运行命令
	// https://github.com/jeessy2/ddns-go/issues/771
	execCmd = exec.Command("bash", "-c", cmd)
	out, err = execCmd.CombinedOutput()

	if err != nil {
		log.Printf("获取%s结果失败! 未能成功执行命令:%s,错误:%q,退出状态码:%s\n", addrType, execCmd.String(), out, err)
		return ""
	}

} else {
	log.Printf("获取%s结果失败! 未能成功执行命令:%s,错误:%q,退出状态码:%s\n", addrType, execCmd.String(), out, err)
	return ""
}

@jeessy2
Copy link
Owner

jeessy2 commented Jul 19, 2023

考虑怎么改,要不要Restricted?

@WaterLemons2k
Copy link
Contributor Author

WaterLemons2k commented Jul 19, 2023

考虑怎么改,要不要Restricted?

要么彻底抛弃,要么只在不支持 Restricted 的 Shell 中回退。

@jeessy2
Copy link
Owner

jeessy2 commented Jul 19, 2023

彻底抛弃吧。

@jeessy2 jeessy2 merged commit 89363a7 into jeessy2:master Jul 19, 2023
2 checks passed
@WaterLemons2k WaterLemons2k deleted the bash branch July 19, 2023 09:00
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