[MPlayer-dev-eng] http to https redirects (was: offer to include gopher protocol support)

[Alexander Strasser]

On 2020-01-23 21:33 +0100, Alexander Strasser wrote:
> On 2020-01-19 17:40 +0100, Alexander Strasser wrote:
> > Hi Lauri!
> >
> > On 2020-01-19 09:39 +0200, Lauri Kasanen wrote:
> > > On Sat, 18 Jan 2020 22:04:41 +0100
> > > Alexander Strasser <eclipse7 at gmx.net> wrote:
> > >
> > > > After looking at that list I noticed https is in there, since some time.
> > > > It makes sense as AFAIK we don't support it with MPlayer's http modules,
> > > > though it seems a bit unexpected from a user's end, that consuming a
> > > > stream, even the "same stream", via https will switch the underlying
> > > > implementation.
> > >
> > > This has the downside that http-to-https redirects don't work.
> > >
> > > mplayer http://...
> > > Unsupported http 301 redirect to https protocol
> > >
> > > While direct https:// plays.
> >
> > That's a good point! I didn't even think about the redirection use case.
> >
> > I fear redirection with stream_ffmpeg might not work or not work for all
> > protocols. That would need to be tested for http/https and if necessary
> > implemented.
>
> I tested redirections with stream_ffmpeg and it seems to work fine.
>
>
> > Then we could IMHO make stream_ffmpeg the preferred handler for
> > http/https. We should compare the implementations a bit deeper to
> > decide if there are enough benefits to keep our http there in the
> > long run.
>
> While testing I found out, that http -> https redirection works fine
> without changing the priority of the stream modules. If it's not an
> mms and not an http redirect, the http stream module fails, and the
> ones after it get a chance to handle the stream.
>
> So on the one hand the attached patch should make http to https work,
> OTOH it will mean at least one more request to the server is done. If
> there is a chain of redirects
>
>     http -> http -> http -> https
>
> or similar, even more requests will be repeated.
>
> Not really sure this matters much, but it could be one reason to put
> stream_ffmpeg before other stream modules handling http in the list.
>
> As always comments welcome!

In addition or instead of the patch below, the patch *attached* to this
mail solves the http -> https redirect failure too. In contrast to
the previous solution, this directly hands over the new URL without
failing in the stream http module.

If we decide to also commit the previous patch (the one quoted below),
that would have the advantage, that stream_ffmpeg would try to open a
http URL that has failed with the previous stream modules. Not sure how
often that will be the case and actually work in stream_ffmpeg, but it
might be worth a try.

Testing, comments and opinions on whether to include either or both
patches welcome.


  Alexander


> From c35b4c917bd48a95f49c1c245e11ec93e1ed623b Mon Sep 17 00:00:00 2001
> From: Alexander Strasser <eclipse7 at gmx.net>
> Date: Thu, 23 Jan 2020 20:23:20 +0100
> Subject: [PATCH] stream_ffmpeg: Handle HTTP protocol too
>
> This change doesn't usually affect the stream module choice for http,
> because the other http handling protocols will be tried first.
>
> Making stream_ffmpeg handle http too, particularly fixes the use case
> where a http connection redirects to an https URL.
>
> While a redirect from http to https will be handled by stream_ffmpeg
> now, the reversed redirect from https to http will not be handled by
> the other stream modules in MPlayer because FFmpeg's http(s) client
> handles redirects internally.
> ---
>  stream/stream_ffmpeg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/stream/stream_ffmpeg.c b/stream/stream_ffmpeg.c
> index cfb8e607f..a57dc36d6 100644
> --- a/stream/stream_ffmpeg.c
> +++ b/stream/stream_ffmpeg.c
> @@ -158,7 +158,7 @@ const stream_info_t stream_info_ffmpeg = {
>    "",
>    "",
>    open_f,
> -  { "ffmpeg", "rtmp", "rtsp", "https", "gopher", NULL },
> +  { "ffmpeg", "rtmp", "rtsp", "http", "https", "gopher", NULL },
>    NULL,
>    1 // Urls are an option string
>  };
> --
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-stream-http-Handover-redirects-to-HTTPS-to-other-str.patch
Type: text/x-diff
Size: 1194 bytes
Desc: not available
URL: <https://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20200125/55f90d70/attachment.patch>