[PATCH] Add support for login/password in http_proxy env variable
Hi, http_proxy:// syntax now well supports proxy authentication, but not in the env variable. So here is a patch to fix that. -- Clément B. Not sent from a jesusPhone.
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote:
Hi,
http_proxy:// syntax now well supports proxy authentication, but not in the env variable. So here is a patch to fix that.
Ping? -- Clément B. Not sent from a jesusPhone.
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote:
+ if (proxy->username && proxy->password) + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", + proxy->username, proxy->password, proxy->hostname, + proxy->port, host_url); + else + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", + proxy->hostname, proxy->port, host_url);
I think some other code supported also setting only username. Also is port certain to be valid (!= 0) here?
On Sat, Nov 27, 2010 at 11:33:28AM +0100, Reimar Döffinger wrote:
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote:
+ if (proxy->username && proxy->password) + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", + proxy->username, proxy->password, proxy->hostname, + proxy->port, host_url); + else + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", + proxy->hostname, proxy->port, host_url);
I think some other code supported also setting only username.
Updated.
Also is port certain to be valid (!= 0) here?
Not it's set to 0 (this is the current behaviour) and handled later. I don't really like this method but changing it belongs to another commit. -- Clément B. Not sent from a jesusPhone.
On Sun, Nov 28, 2010 at 12:01:16AM +0100, Clément Bœsch wrote:
On Sat, Nov 27, 2010 at 11:33:28AM +0100, Reimar Döffinger wrote:
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote:
+ if (proxy->username && proxy->password) + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", + proxy->username, proxy->password, proxy->hostname, + proxy->port, host_url); + else + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", + proxy->hostname, proxy->port, host_url);
I think some other code supported also setting only username.
Updated.
Also is port certain to be valid (!= 0) here?
Not it's set to 0 (this is the current behaviour) and handled later. I don't really like this method but changing it belongs to another commit.
OK to commit? -- Clément B. Not sent from a jesusPhone.
On Mon, Nov 29, 2010 at 11:09:40PM +0100, Clément Bœsch wrote:
On Sun, Nov 28, 2010 at 12:01:16AM +0100, Clément Bœsch wrote:
On Sat, Nov 27, 2010 at 11:33:28AM +0100, Reimar Döffinger wrote:
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote:
+ if (proxy->username && proxy->password) + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", + proxy->username, proxy->password, proxy->hostname, + proxy->port, host_url); + else + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", + proxy->hostname, proxy->port, host_url);
I think some other code supported also setting only username.
Updated.
Also is port certain to be valid (!= 0) here?
Not it's set to 0 (this is the current behaviour) and handled later. I don't really like this method but changing it belongs to another commit.
OK to commit?
No news so I suppose it's ok; i'll commit this at the end of the week end if there is not objection. -- Clément B. Not sent from a jesusPhone.
On Fri, Dec 03, 2010 at 08:53:48PM +0100, Clément Bœsch wrote:
On Mon, Nov 29, 2010 at 11:09:40PM +0100, Clément Bœsch wrote:
On Sun, Nov 28, 2010 at 12:01:16AM +0100, Clément Bœsch wrote:
On Sat, Nov 27, 2010 at 11:33:28AM +0100, Reimar Döffinger wrote:
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote:
+ if (proxy->username && proxy->password) + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", + proxy->username, proxy->password, proxy->hostname, + proxy->port, host_url); + else + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", + proxy->hostname, proxy->port, host_url);
I think some other code supported also setting only username.
Updated.
Also is port certain to be valid (!= 0) here?
Not it's set to 0 (this is the current behaviour) and handled later. I don't really like this method but changing it belongs to another commit.
OK to commit?
No news so I suppose it's ok; i'll commit this at the end of the week end if there is not objection.
You should maybe send your updated patch first, I think the email where you said "Updated" contained no attachment...
On Fri, Dec 03, 2010 at 09:33:54PM +0100, Reimar Döffinger wrote:
On Fri, Dec 03, 2010 at 08:53:48PM +0100, Clément Bœsch wrote:
On Mon, Nov 29, 2010 at 11:09:40PM +0100, Clément Bœsch wrote:
On Sun, Nov 28, 2010 at 12:01:16AM +0100, Clément Bœsch wrote:
On Sat, Nov 27, 2010 at 11:33:28AM +0100, Reimar Döffinger wrote:
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote:
+ if (proxy->username && proxy->password) + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", + proxy->username, proxy->password, proxy->hostname, + proxy->port, host_url); + else + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", + proxy->hostname, proxy->port, host_url);
I think some other code supported also setting only username.
Updated.
Also is port certain to be valid (!= 0) here?
Not it's set to 0 (this is the current behaviour) and handled later. I don't really like this method but changing it belongs to another commit.
OK to commit?
No news so I suppose it's ok; i'll commit this at the end of the week end if there is not objection.
You should maybe send your updated patch first, I think the email where you said "Updated" contained no attachment...
It had attachment (I just checked). Note that mailman seems completely broken with attachments so you can't trust mplayer-dev-eng archives for this. Anyway, patch is reattached to this mail with minor simplifications to ease review. After this patch, what is left to fix is mainly making all this stuff to work with -user and -passwd (at the moment it only works if the destination server has authentication in URL when used with a proxy), and update the documentation. I also remember you talking about an ipv6 ugliness/issue, could you tell me what was wrong? Anyway, thanks again for reviewing this. -- Clément B. Not sent from a jesusPhone.
On Fri, Dec 03, 2010 at 09:55:56PM +0100, Clément Bœsch wrote:
On Fri, Dec 03, 2010 at 09:33:54PM +0100, Reimar Döffinger wrote:
On Fri, Dec 03, 2010 at 08:53:48PM +0100, Clément Bœsch wrote:
On Mon, Nov 29, 2010 at 11:09:40PM +0100, Clément Bœsch wrote:
On Sun, Nov 28, 2010 at 12:01:16AM +0100, Clément Bœsch wrote:
On Sat, Nov 27, 2010 at 11:33:28AM +0100, Reimar Döffinger wrote:
On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote: > + if (proxy->username && proxy->password) > + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", > + proxy->username, proxy->password, proxy->hostname, > + proxy->port, host_url); > + else > + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", > + proxy->hostname, proxy->port, host_url);
I think some other code supported also setting only username.
Updated.
Also is port certain to be valid (!= 0) here?
Not it's set to 0 (this is the current behaviour) and handled later. I don't really like this method but changing it belongs to another commit.
OK to commit?
No news so I suppose it's ok; i'll commit this at the end of the week end if there is not objection.
You should maybe send your updated patch first, I think the email where you said "Updated" contained no attachment...
It had attachment (I just checked). Note that mailman seems completely broken with attachments so you can't trust mplayer-dev-eng archives for this. Anyway, patch is reattached to this mail with minor simplifications to ease review.
Sorry, I looked at the wrong email. I think it should be ok.
I also remember you talking about an ipv6 ugliness/issue, could you tell me what was wrong?
The escaping code has a special-case so it would not clobber litteral IPv6 addresses. But that code will not work for e.g. http_proxy://proxy/http://[<ipv6-address>] since it will destroy the [] around the ipv6 address in that case.
On Fri, Dec 03, 2010 at 11:34:09PM +0100, Reimar Döffinger wrote:
On Fri, Dec 03, 2010 at 09:55:56PM +0100, Clément Bœsch wrote:
On Fri, Dec 03, 2010 at 09:33:54PM +0100, Reimar Döffinger wrote:
On Fri, Dec 03, 2010 at 08:53:48PM +0100, Clément Bœsch wrote:
On Mon, Nov 29, 2010 at 11:09:40PM +0100, Clément Bœsch wrote:
On Sun, Nov 28, 2010 at 12:01:16AM +0100, Clément Bœsch wrote:
On Sat, Nov 27, 2010 at 11:33:28AM +0100, Reimar Döffinger wrote: > On Wed, Nov 24, 2010 at 08:34:40PM +0100, Clément Bœsch wrote: > > + if (proxy->username && proxy->password) > > + return snprintf(dst, dst_size, "http_proxy://%s:%s@%s:%d/%s", > > + proxy->username, proxy->password, proxy->hostname, > > + proxy->port, host_url); > > + else > > + return snprintf(dst, dst_size, "http_proxy://%s:%d/%s", > > + proxy->hostname, proxy->port, host_url); > > I think some other code supported also setting only username.
Updated.
> Also is port certain to be valid (!= 0) here?
Not it's set to 0 (this is the current behaviour) and handled later. I don't really like this method but changing it belongs to another commit.
OK to commit?
No news so I suppose it's ok; i'll commit this at the end of the week end if there is not objection.
You should maybe send your updated patch first, I think the email where you said "Updated" contained no attachment...
It had attachment (I just checked). Note that mailman seems completely broken with attachments so you can't trust mplayer-dev-eng archives for this. Anyway, patch is reattached to this mail with minor simplifications to ease review.
Sorry, I looked at the wrong email. I think it should be ok.
Applied.
I also remember you talking about an ipv6 ugliness/issue, could you tell me what was wrong?
The escaping code has a special-case so it would not clobber litteral IPv6 addresses. But that code will not work for e.g. http_proxy://proxy/http://[<ipv6-address>] since it will destroy the [] around the ipv6 address in that case.
Mmh ok, I'll have a look. Thanks. -- Clément B. Not sent from a jesusPhone.
participants (2)
-
Clément Bœsch -
Reimar Döffinger