Re: http to https redirects (was: offer to include gopher protocol support)

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 <[email protected]> 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 <[email protected]>
> 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
>  };
> --

_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng

From f767ecf3a39ef8e5e3cb9600b39b4596bc1b610b Mon Sep 17 00:00:00 2001
From: Alexander Strasser <[email protected]>
Date: Sat, 25 Jan 2020 00:10:54 +0100
Subject: [PATCH] stream/http: Handover redirects-to-HTTPS to other stream
 modules

As we do not support HTTPS in our stream http module, allow to
smoothly handover the HTTPS location to another stream module.
At this time that other module would probably be stream_ffmpeg.

Before this change we would, e.g. in stream_ffmpeg, start again
at the initial URL and likely follow again all redirects.
---
 stream/http.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stream/http.c b/stream/http.c
index 3baefd2e5..5081886cb 100644
--- a/stream/http.c
+++ b/stream/http.c
@@ -828,7 +828,7 @@ static int http_streaming_start(stream_t *stream, int* file_format) {
 				if( next_url!=NULL ) {
 					int is_ultravox = strcasecmp(stream->streaming_ctrl->url->protocol, "unsv") == 0;
 					stream->streaming_ctrl->url = url_redirect( &url, next_url );
-					if (url_is_protocol(url, "mms")) {
+					if (url_is_protocol(url, "https") || url_is_protocol(url, "mms")) {
 						res = STREAM_REDIRECTED;
 						goto err_out;
 					}
--