[MPlayer-dev-eng] [PATCH 1/7] Unescape login/password before base64 encode

[Reimar Döffinger]

On Wed, Nov 10, 2010 at 08:50:28PM +0100, Clément Bœsch wrote:
> On Wed, Nov 10, 2010 at 08:35:46PM +0100, Reimar Döffinger wrote:
> > On Wed, Nov 10, 2010 at 08:02:14PM +0100, Clément Bœsch wrote:
> > > On Sun, Nov 07, 2010 at 11:31:50PM +0100, Clément Bœsch wrote:
> > > > On Mon, Oct 18, 2010 at 12:05:51PM +0200, Clément Bœsch wrote:
> > > > > I already sent it in a previous mail: it allows login/password to be
> > > > > unescaped before base64 encode so auth with special characters now
> > > > > works.
> > > > 
> > > > Al'right, a new version of this patch: while the form
> > > > 
> > > >   mplayer 'http://u:p@host'
> > > > 
> > > > was url encoded,
> > > > 
> > > >   mplayer 'http://host' -user u -passwd p
> > > > 
> > > > wasn't.
> > > > 
> > > > Now it's fixed.
> > > > 
> > > > Reimar, you also mentionned the url escaping may be broken, what was the
> > > > issue?
> > > > 
> > > > Anyway, is this version fine?
> > > > 
> > > 
> > > If anyone has something against this, speak now or I'll commit this one it
> > > in the next days.
> > 
> > I have the suspicion that this is wildly escaping and unescaping until
> > it just happens to work without any real concept.
> > Why does url_new not just apply escaping only to the path part?
> 
> As I said the first time, I started to make a patch to update the
> escaping, but it much more complicated and risky; imagine you simply have
> to parse: http_proxy://foo:bar@host:4321/http://xxx:yyy@proxy:1234. There
> is some cases not easy to handle.

And what about unescaping in url_new when username/password is assigned?

> > I don't think escaping is supposed to be applied to anything else.
> 
> We could also have users who try to make special character by urlencoding
> themselves the password (special char not easy to escape with the shell,
> or simply break url parsing in MPlayer because of ':' or '@' in it).
> MPlayer urlencode won't change the string, but the http auth code will be
> able to decode it.

Well, thinking more about it I have the suspicion that your patch will
actually break e.g. %20 as password, it well end up using a single space
as password (unless you pre-escape it of course)...
Doing it in url_new would at least allow the other method of specifying
it...
I'd actually be quite curious what webbrowser do in such a case.