send smtp email through a proxy

[qdongxu]

This PR addresses the issue #27348.
The notification mailing will leverage the environment variable, ALL_PROXY if it is exported.

[wxiaoguang]

"modules/proxy" should be improved, instead of introducing a totally new mechanism.

[lunny]

Looks like it's not finished.

[silverwind]

1. Uppercase proxy vars are discouraged as per this, lowercase should be the only supported variant.
2. I'm worried about all_proxy taking precedence over http_proxy and https_proxy once net/http: have DefaultTransport support all_proxy (SOCKS5) by default golang/go#31813 is implemented. Shouldn't it be called smtp_proxy?

[qdongxu]

Looks like it's not finished.

It's finished. the golang.org/x/net/proxy package support ALL_PROXY/all_proxy environment variable out of the box.

[qdongxu]

according to @wxiaoguang and @silverwind's comments, the strategy is to configure proxies in app.ini prior to the shell environment variable in order to behave clearly. the modules/proxy handles http_proxy only for git operations, but stmp can only leverage socks proxy. and it's a separate and irrelative decision to proxy git or mail. I'd like to add a [smtp_proxy] section, analogizing the [proxy] section.

[silverwind]

A specific ini option sound much better then relying on all_proxy, which seems a too dangerous variable beause of it's possible influence on other protocols like http.

Apparently there is already a known bug in golang related to the variable: golang/go#16715

[wxiaoguang]

but stmp can only leverage socks proxy.

That's not true. HTTP CONNECT proxy can also be used for any TCP protocol.

and it's a separate and irrelative decision to proxy git or mail.

From a system design view, the Gitea should have the consistent proxy behavior.

I'd like to add a [smtp_proxy] section, analogizing the [proxy] section.

Although at the moment I won't say "block" for it, but I really don't like this strange (inconsistent) behavior.

[wxiaoguang]

Again, such design doesn't look good to me. Could you explain why the [proxy] couldn't be reused? From my understanding, it should work (by making some improvements) with SMTP.

[qdongxu]

1. about @silverwind 's commnet, add [stmp_proxy]: enabled property. proxy will only take effect when it is set to true. either the [stmp_proxy]:proxy_url or the all_prxoy environment will be ignore when [stmp_proxy]: enabled is false.

2. about @silverwind 's concern, all_proxy environment variables may impact other protocols, this will not happen as I checked the going library, package golang.org/x/net/http/httpproxy, which only reads http_proxy/https_proxy environment variables, while package golang.org/x/[email protected]/proxy, reads all_proxy only. there would be no conflict.

3. about @wxiaoguang's comment http CONNECT can proxy for any TCP traffic, that's true after the connection is established, but the CONNECT here is a http method CONNECT instead of a TCP connect invocation. for the moment I evaluated golang proxy( socks5 proxy only ), golang httpproxy ( bind to http.Client) and https://github.com/elazarl/goproxy , I did not find the API to bind httpproxy for a non-http traffic, (while a socks proxy is for any tcp traffic, including a http traffic). maybe I am wrong but a relevant reference or example would be appreciated.

4. about @wxiaoguang's comment, the Gitea should have consistent proxy behavior. I agree on it. but according to (4), the most consistent way is to make the properties in [proxy] and [smtp] section look similar and behave similarly.

5. In [proxy] there's a ProxyHosts property, if host in ProxyHosts use proxyURL from config file , otherwise use http_proxy from the environment variable( and implies using the no_proxy environment variable ). This looks confusing, it's better to follow a unified http_proxy/no_proxy rule, but this can be addressed in another issue.

[wxiaoguang]

I am pretty sure implementing the HTTP CONNECT proxy support is the right approach, and it's not difficult, like this: https://gist.github.com/jim3ma/3750675f141669ac4702bc9deaf31c6b .

HTTP CONNECT proxy is exactly (almost) the same as SOCKS5H proxy.

[qdongxu]

modules/proxy/httpsdialer.go is owned to https://gist.github.com/jim3ma/3750675f141669ac4702bc9deaf31c6b

updated to reuse [proxy]. and add a property SMTP_PROXY_ENABLED default as false, for backward compatibility.

[wxiaoguang]

The Gitea's proxy system has many legacy problems.

I vote my approval here and will do some improvements in following PRs.

[silverwind]

Still lint failures.

[silverwind]

Seems we could use httplib.NewRequest instead, which also brings the benefit of setting a common user-agent.

[qdongxu]

Sorry for being late. As I check the httplib.NewRequest, It aims to wrap a normal HTTP request and it uses the HTTP proxy environment variable internally :

		TLSClientConfig: r.setting.TLSClientConfig,
		Proxy:           http.ProxyFromEnvironment,

while herein the proxy/httpsdialer.go just make a CONNECT to the proxy per se.

[qdongxu]

An alternative solution works : https://github.com/ankraft/smtpproxy
the issue and pr was closed thereby. thanks everyone reviewing the solution and code anyway.