Hi, Holyfoot! On Sep 21, holyfoot@askmonty.org wrote:

revision-id: d3f10e30ec39bccf1172c7b2eb159f40e412bdfd (mariadb-10.0.21-14-gd3f10e3) parent(s): abd31ca2b6cde024772fcac0f4953219d76c6344 committer: Alexey Botchkov timestamp: 2015-09-21 14:06:15 +0500 message:

MDEV-8431 Feedback plugin needs an option for http proxy. 'feedback_proxy_server' system variable added to specify the proxy server. Not a dynamic one.

How did you test this feature?

diff --git a/plugin/feedback/feedback.cc b/plugin/feedback/feedback.cc index f644bd5..f76c52a 100644 --- a/plugin/feedback/feedback.cc +++ b/plugin/feedback/feedback.cc @@ -250,6 +251,9 @@ static int init(void *p)

prepare_linux_info();

+ proxy_url= proxy_server ? + Url::create(proxy_server, strlen(proxy_server)) : 0;

Some diagnostics would be nice here. At least "feedback: invalid proxy url: '%.*s'". See how ivalid feedback urls are reported.

+ url_count= 0; if (*url) { @@ -347,6 +354,9 @@ static MYSQL_SYSVAR_ULONG(send_timeout, send_timeout, PLUGIN_VAR_RQCMDARG, static MYSQL_SYSVAR_ULONG(send_retry_wait, send_retry_wait, PLUGIN_VAR_RQCMDARG, "Wait this many seconds before retrying a failed send.", NULL, NULL, 60, 1, 60*60*24, 1); +static MYSQL_SYSVAR_STR(proxy_server, proxy_server, + PLUGIN_VAR_READONLY | PLUGIN_VAR_RQCMDARG, + "Proxy server address.", NULL, NULL,0);

Mention that it must be an url. "Proxy server URL" or "Proxy server address, in the correct URI format", or something.

static struct st_mysql_sys_var* settings[] = { MYSQL_SYSVAR(server_uid), diff --git a/plugin/feedback/url_http.cc b/plugin/feedback/url_http.cc index f214f7a..eade67c 100644 --- a/plugin/feedback/url_http.cc +++ b/plugin/feedback/url_http.cc @@ -148,9 +148,12 @@ int Url_http::send(const char* data, size_t data_length) my_socket fd= INVALID_SOCKET; char buf[1024]; uint len= 0; + Url_http *proxy_loc= (Url_http *) proxy_url;

addrinfo *addrs, *addr, filter= {0, AF_UNSPEC, SOCK_STREAM, 6, 0, 0, 0, 0}; - int res= getaddrinfo(host.str, port.str, &filter, &addrs); + int res= proxy_url ? + getaddrinfo(proxy_loc->host.str, proxy_loc->port.str, &filter, &addrs) : + getaddrinfo(host.str, port.str, &filter, &addrs);

First, you don't know whether proxy_url is Url_http. It might be easier to have an option --feedback-http-proxy=host:port (not the url). It also solves the potential future confusion if we'll have more than one transport. Second, I don't think proxies work this way at all, for a http proxy, you need to specify the host in th GET request, like POST http://foo.bar/bla/bla So, I think, a better implementation would be to take proxy into account not in Url_http::send(), but already in http_create(). And create Url_http objects for feedback url with proxy's host and port, and with the appropriately adjusted path. Regards, Sergei