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

toxic dde proxy environmental variable sync #15

Closed
DuckSoft opened this issue May 10, 2020 · 27 comments · Fixed by #28
Closed

toxic dde proxy environmental variable sync #15

DuckSoft opened this issue May 10, 2020 · 27 comments · Fixed by #28
Assignees

Comments

@DuckSoft
Copy link

DuckSoft commented May 10, 2020

🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️

intro

before stating what the hug was going on, i want to ask everyone an interesting question:

first of all, i have an HTTP proxy on 127.0.0.1:8888, and an HTTPS proxy on example.com:443.
are the following configuration cases correct or not, why and why not?

  1. export ftp_proxy=http://127.0.0.1:8888
  2. export https_proxy=http://127.0.0.1:8888
  3. export http_proxy=https://example.com:443

i want to say that, those are all correct configurations.

🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️

problem

let's look at the problem itself:
file /proxy/proxy.go is used for deepin desktop environment to sync gsettings proxy entries into environmental variables, which looks pretty good. but if you look deeper, you will get something unusual:

go-lib/proxy/proxy.go

Lines 145 to 159 in 5406dea

case proxyModeManual:
doSetEnv(envHttpProxy, getProxyValue(proxyTypeHttp))
doSetEnv(envHttpsProxy, getProxyValue(proxyTypeHttps))
doSetEnv(envFtpProxy, getProxyValue(proxyTypeFtp))
doSetEnv(envSocksProxy, getProxyValue(proxyTypeSocks))
arrayIgnoreHosts := proxySettings.GetStrv(gkeyProxyIgnoreHosts)
ignoreHosts := strings.Join(arrayIgnoreHosts, ",")
doSetEnv(envNoProxy, ignoreHosts)
// fallback socks proxy value to http to be compatible with Qt>=4.6
if utils.IsEnvExists(envSocksProxy) && !utils.IsEnvExists(envHttpProxy) {
doSetEnv(envHttpProxy, os.Getenv(envSocksProxy))
}
}

if you look into getProxyValue, you will see that, the program here assumes http_proxy can only use http schemas, and the same for https and ftp.

proxyValue = fmt.Sprintf("%s://%s:%s", proxyType, host, port)

if we admit this, we are to say, none of the configuration cases in our intro section is correct, since ftp_proxy used http, https_proxy used http and http_proxy used https.

🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️

discovery

i encountered this problem when a random deepin user reported to Qv2ray, saying that the system proxy isn't working at all. astonished, we have checked in detail, but to find that the $https_proxy environmental variable was incorrectly set.

in his case, Qv2ray have a HTTP proxy at 127.0.0.1:8888. Qv2ray configures system proxy through gsettings, and thus triggered that proxy.go, which is used to sync gsettings into environmental variables. $http_proxy variable was correctly set to http://127.0.0.1:8888, but $https_proxy variable was incorrectly set to https://127.0.0.1:8888. since HTTP proxy can't accept HTTPS requests at all, the victim's browser refused to work whenever he wanted to open a HTTPS site, and that's how the problem was discovered.

🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️

joke

Qv2ray/Qv2ray#589

calendar
SUN MON TUE WED THU FRI SAT
🐛 🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️
🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️
🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️

DOVE OVERFLOW

@ghost
Copy link

ghost commented May 10, 2020

As far as I could tell, the https_proxy environment variable can be even socks5:// scheme.
image

Setting it to plain https without checking is not acceptable.

@ChungZH
Copy link

ChungZH commented May 15, 2020

image

救救孩子吧,自从用了 DDE,只要一设置系统代理,不仅浏览器上不去 HTTPS 的站,更新个系统都会卡死,都要绝望了

🕊️ 🕊️ 🕊️ 🕊️ 🕊️ 🕊️

想挂个代理滚个系统都当场爆毙~
image

@DuckSoft
Copy link
Author

DuckSoft commented Jul 21, 2020

补丁已合并,但上游放出修复过的版本还需要时间。
先关闭了。

@suyar
Copy link

suyar commented Aug 19, 2020

至今还没更新吗,deepin20 beta 截止 2020/08/19 还没解决,还是有同样的问题,救救孩子吧

@muihiuwev
Copy link

till 21 aug, still the same, so help me god.

@CoelacanthusHex
Copy link

@DuckSoft 建议 Reopen, 不然我觉得 deepin 可能会遗忘这个问题……

BTW, I think this patch for so BAD bug should be backported.

@DuckSoft DuckSoft reopened this Aug 23, 2020
@XMuli
Copy link
Member

XMuli commented Aug 24, 2020

补丁已合并,但上游放出修复过的版本还需要时间。

「English:」
Previously, pr was incorrectly submitted to the develop branch (due to the strategy, the branch is shelved), the latest and most comprehensive branch, usually the default branch uos / master branch (recommended merge branch); then this problem, just now, has been with the backend Team communication suggestion, if possible, let users write the complete string themselves (deepin does not do any splicing and verification), and it is ready to be repaired by hand;

Supplement: If it is a pr from github, if it is merged, it is updated synchronously in real time (only the default branch). After a few minutes, you can see the code submitted by contributors in the default branch. If this is not the case, where might it be? What went wrong, you need to troubleshoot it (there are also internal submissions, which may cause some kind of collision~; there is also a manpower problem~);

For this kind of problem that everyone actually needs, the community has always been willing and has always maintained an open and cooperative mentality~ to build a better experience together.


「中文原文:」
先前 pr 都错误的提交到 develop 分支(由于策略,是搁置的分支),最新的最全的分支,通常是默认分支 uos / master 分支(推荐合并分支); 然后这个问题,刚才,已经和后端团队沟通建议,可以的话,让用户自己来写 完整字符串(deepin 不做任何的拼接和校验),已经准备作手修复了;

补充:如果是来自 github 的 pr, 若是被合并,是实时同步更新(仅默认分支),几分钟后,就可以在默认分支看到来自贡献者提交的代码, 若非这种情况,则可能是哪里出了什么问题,需要进行排查一下(还有来自内部的提交,可能会造成某种碰撞~;还有一个人手问题~);

对于这种大家都实际都有所需的问题,社区一直愿意和一直保持开放合作的心态~,一起构建更好的使用体验

@XMuli
Copy link
Member

XMuli commented Aug 31, 2020

Update:

@DuckSoft It has been fixed f3021d13975a [Branch in uos], if there is still a problem, please communicate with @XMuli ; remove this problem for users of deepin or dde which is troublesome

@DuckSoft
Copy link
Author

Appreciate! I'll close this issue and remove that hint in Qv2ray/Qv2ray#589 once the fix is published and confirmed.

Update:

@DuckSoft It has been fixed f3021d13975a [Branch in uos], if there is still a problem, please communicate with @XMuli ; remove this problem for users of deepin or dde which is troublesome

@XMuli
Copy link
Member

XMuli commented Sep 2, 2020

@DuckSoft thanks

@RigoLigoRLC
Copy link

Thanks, this bug is finally fixed here.

DuckSoft added a commit to Qv2ray/Qv2ray that referenced this issue Oct 22, 2020
removing this nag due to: linuxdeepin/go-lib#15 (comment)
big thanks to @XMuli, @RigoLigo, @felixonmars and all other contributors involved!
ghost pushed a commit to Qv2ray/Qv2ray that referenced this issue Oct 23, 2020
removing this nag due to: linuxdeepin/go-lib#15 (comment)
big thanks to @XMuli, @RigoLigo, @felixonmars and all other contributors involved!
@ghost
Copy link

ghost commented Oct 23, 2020

Thanks for your hard work, the removal of the prompts will be released soon in the alpha preview.
By the way, does deepin have any other portal to report bugs? A 5-months-long delay for a simple proxy setting is not what users expected to.

@XMuli
Copy link
Member

XMuli commented Oct 23, 2020

I'm sure many of you are wondering why a simple merge is taking so long, but in the next few days I'll write up the reasons why, and answer any questions you may have.

Fortunately, now that the community version code is up to date and in the latest code, if there are any problems later, feedback and bugs will be fixed much more quickly.

@RigoLigoRLC
Copy link

Haha...it appears that after the upgrade from 20 (1003) to 20.1 (1010), this problem revived ONCE AGAIN! ROFL
image

@RigoLigoRLC
Copy link

RigoLigoRLC commented Dec 30, 2020

Request a re-open for this issue! Someone uncommented the problematic code WITHOUT EVEN READING GIT BLAME!
a1b16c4 This is so hilarious. UnionTech company, what are you doing? I'm genuinely pissed.

Keep this issue open until the upstream push the updates that will fix this issue. AND FIX THE APPSTORE NOT DAEMON!

@DuckSoft DuckSoft reopened this Dec 30, 2020
@RigoLigoRLC
Copy link

RigoLigoRLC commented Dec 30, 2020

I proposed a change in deepin go-lib: to set the envs correctly, not the "how not to set proxy server" way in legacy code.
#26

@XMuli
Copy link
Member

XMuli commented Dec 31, 2020

Request a re-open for this issue! Someone uncommented the problematic code WITHOUT EVEN READING GIT BLAME!
a1b16c4 This is so hilarious. UnionTech company, what are you doing? I'm genuinely pissed.

Keep this issue open until the upstream push the updates that will fix this issue. AND FIX THE APPSTORE NOT DAEMON!

I understand how you feel. A seemingly retarded action has its reasons.
This commit is a reluctant decision, for which see Qv2ray 检测到 DDE 环境,提示弹窗:Deepin 设置代理错误

This web proxy section, which is already being redesigned and implemented

@RigoLigoRLC
Copy link

Request a re-open for this issue! Someone uncommented the problematic code WITHOUT EVEN READING GIT BLAME!
a1b16c4 This is so hilarious. UnionTech company, what are you doing? I'm genuinely pissed.

Keep this issue open until the upstream push the updates that will fix this issue. AND FIX THE APPSTORE NOT DAEMON!

I understand how you feel. A seemingly retarded action has its reasons.
This commit is a reluctant decision, for which see Qv2ray 检测到 DDE 环境,提示弹窗:Deepin 设置代理错误

This web proxy section, which is already being redesigned and implemented

Okay. But why they're not doing as what I proposed in the pull request?

That should be a proper fix for the app store problem.

@DuckSoft DuckSoft changed the title poisonous dde proxy environmental variable sync toxic dde proxy environmental variable sync Jan 18, 2021
justforlxz added a commit to justforlxz/go-lib that referenced this issue Feb 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use
http protocol.

Issues: Closes linuxdeepin#15
Log:
Change-Id: I5c2f5c384ff8266b10840ff78ca9710543cf38fb
justforlxz added a commit to justforlxz/go-lib that referenced this issue Feb 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use
http protocol.

Issues: Closes linuxdeepin#15
Log:
justforlxz added a commit to justforlxz/go-lib that referenced this issue Mar 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.

Issues: Closes linuxdeepin#15
Log:
justforlxz added a commit to justforlxz/go-lib that referenced this issue Mar 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.

Issue: Closes linuxdeepin#15
Log:
justforlxz added a commit to justforlxz/go-lib that referenced this issue Mar 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.

Issue: Closes linuxdeepin#15
Log:
justforlxz added a commit to justforlxz/go-lib that referenced this issue Mar 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.
about: qv2ray

Issue: Closes linuxdeepin#15
Log:
justforlxz added a commit to justforlxz/go-lib that referenced this issue Mar 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.
I don't want to continue talking about this bug. If the problem still occurs, I will review the behavior of the relevant person in charge and make certain punishments.
about: qv2ray
- Qv2ray/Qv2ray#1278
- Qv2ray/Qv2ray#1057
- Qv2ray/Qv2ray#913
- Qv2ray/Qv2ray#915
- Qv2ray/Qv2ray#863

Issue: Closes linuxdeepin#15
Log:
deepinAdmin pushed a commit that referenced this issue Mar 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.
I don't want to continue talking about this bug. If the problem still occurs, I will review the behavior of the relevant person in charge and make certain punishments.
about: qv2ray
- Qv2ray/Qv2ray#1278
- Qv2ray/Qv2ray#1057
- Qv2ray/Qv2ray#913
- Qv2ray/Qv2ray#915
- Qv2ray/Qv2ray#863

Issue: Closes #15
Log:
deepinAdmin pushed a commit that referenced this issue Mar 22, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.
I don't want to continue talking about this bug. If the problem still occurs, I will review the behavior of the relevant person in charge and make certain punishments.
about: qv2ray
- Qv2ray/Qv2ray#1278
- Qv2ray/Qv2ray#1057
- Qv2ray/Qv2ray#913
- Qv2ray/Qv2ray#915
- Qv2ray/Qv2ray#863

Issue: Closes #15
Log:
Change-Id: I1782759252d57cd54d2e051d416d35c28e21e3f9
@DuckSoft
Copy link
Author

Finally! Thank you all!

deepinAdmin pushed a commit that referenced this issue May 27, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.
I don't want to continue talking about this bug. If the problem still occurs, I will review the behavior of the relevant person in charge and make certain punishments.
about: qv2ray
- Qv2ray/Qv2ray#1278
- Qv2ray/Qv2ray#1057
- Qv2ray/Qv2ray#913
- Qv2ray/Qv2ray#915
- Qv2ray/Qv2ray#863

Issue: Closes #15
Log:
Change-Id: I1782759252d57cd54d2e051d416d35c28e21e3f9
deepinAdmin pushed a commit that referenced this issue Jun 1, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.
I don't want to continue talking about this bug. If the problem still occurs, I will review the behavior of the relevant person in charge and make certain punishments.
about: qv2ray
- Qv2ray/Qv2ray#1278
- Qv2ray/Qv2ray#1057
- Qv2ray/Qv2ray#913
- Qv2ray/Qv2ray#915
- Qv2ray/Qv2ray#863

Issue: Closes #15
Log:
Change-Id: I1782759252d57cd54d2e051d416d35c28e21e3f9
deepinAdmin pushed a commit that referenced this issue Jun 1, 2021
keep consisent with gnome, http_proxy/https_proxy/ftp_proxy should use http protocol.
I don't want to continue talking about this bug. If the problem still occurs, I will review the behavior of the relevant person in charge and make certain punishments.
about: qv2ray
- Qv2ray/Qv2ray#1278
- Qv2ray/Qv2ray#1057
- Qv2ray/Qv2ray#913
- Qv2ray/Qv2ray#915
- Qv2ray/Qv2ray#863

Issue: Closes #15
Log:
Change-Id: I1782759252d57cd54d2e051d416d35c28e21e3f9
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 a pull request may close this issue.