[vlc-devel] [PATCH 1/2] http2: fix up URLs
Rémi Denis-Courmont
remi at remlab.net
Fri Oct 9 10:34:59 CEST 2020
Hi,
#3435 does not specify the context, and I'm not going to ask the notoriously troll-prone Slackware VLC maintainer what they meant 11 years ago. The far most common occurrences of unencoded URL white spaces are playlist files (in a large sense) and redirects. Both are already handled. As far as I am concerned, the bug is fixed.
I don't even see what filesystems have to do with this. The directory access modules are perfectly capable of encoding file paths correctly from the very beginning.
The only real problem that I know, is the UI dialog which rejects invalid chars. And this patchset doesn't address that.
Le 9 octobre 2020 11:23:29 GMT+03:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
>Hi,
>
>On Fri, Oct 09, 2020 at 10:01:36AM +0200, Thomas Guillem wrote:
>> "VLC should automatically replace spaces with %20 like all browsers
>do"
>>
>> It's not the case when I type: ./vlc "https://server/file name.mp3"
>>
>> So, how can you say that #3435 is fixed?
>>
>> You are right about one point, it should not be done in http modules,
>but at a higher level when we know the url come from an user.
>
>If it comes from the user, shouldn't it be fixed in the interface?
>(which
>would be the only point where the URL would be fixed up, including non
>interactive inputs interface like playlist demuxers).
>
>A note here: fixing url in one way will always break some other invalid
>use cases that happened to work before. That's a non-issue because they
>are invalid anyway but we should probably focus on the use case that
>actually brings UX improvement like the space change, which happens
>regularily if you have a list of files to play from a server and get
>the name (not encoded) from the FS.
>
>Regards,
>--
>Alexandre Janniaux
>Videolabs
>
>> On Fri, Oct 9, 2020, at 09:59, Rémi Denis-Courmont wrote:
>> > Hi,
>> >
>> > #3435 is fixed. VLC can parse file paths and insufficiently encoded
>URLs from playlists or redirects just fine. We have to do the fixup
>there in the producer so that relative-to-absolute resolution works.m
>properly.
>> >
>> > Validation must be done in access because it can't be done earlier
>due to the different rules for different MRL schemes.
>> >
>> > Le 9 octobre 2020 10:41:28 GMT+03:00, Thomas Guillem
><thomas at gllm.fr> a écrit :
>> >> I remember a previous discussion where we said it was the
>responsibility of the access modules to do this fixup.
>> >>
>> >> cf. https://trac.videolan.org/vlc/ticket/18991 and
>https://git.videolan.org/?p=vlc.git;a=commit;h=762ca1e8a01278b34ddb34765f3339690aad5d2e
>> >>
>> >> I'm not sure that URLs are fixed up at this point.
>> >>
>> >> How do you propose to fix #3435
><https://trac.videolan.org/vlc/ticket/3435> then? Because you said it
>was fixed but it is *not*. VLC can't handle unvalid http URLs with a "
>". Every other player or browsers can.
>> >>
>> >> On Fri, Oct 9, 2020, at 09:26, Rémi Denis-Courmont wrote:
>> >>> URLs are already fixed up when they get to this point. Fixing up
>URLs twice is a recipe for disaster. We've tried.
>> >>>
>> >>> -1
>> >>>
>> >>> Le 9 octobre 2020 09:56:58 GMT+03:00, Thomas Guillem
><thomas at gllm.fr> a écrit :
>> >>>> This will allow VLC to open unvalid URLs (using a space instead
>of the
>> >>>> %20 encoding, for example). Every other browsers or players are
>doing
>> >>>> this fixup.
>> >>>>
>> >>>> Fixes #3435 modules/access/http/access.c | 12 ++++++++++--
>> >>>> 1 file changed, 10 insertions(+), 2 deletions(-)
>> >>>>
>> >>>> diff --git a/modules/access/http/access.c
>b/modules/access/http/access.c
>> >>>> index a3805d0d426..123daaf51d3 100644
>> >>>> --- a/modules/access/http/access.c
>> >>>> +++ b/modules/access/http/access.c
>> >>>> @@ -171,19 +171,27 @@ static int Open(vlc_object_t *obj)
>> >>>> struct vlc_url_t crd_url;
>> >>>> char *psz_realm = NULL;
>> >>>>
>> >>>> - vlc_UrlParse(&crd_url, access->psz_url);
>> >>>> + char *url_fixup = vlc_uri_fixup(access->psz_url);
>> >>>> + if (url_fixup == NULL)
>> >>>> + goto error;
>> >>>> +
>> >>>> + vlc_UrlParse(&crd_url, url_fixup);
>> >>>> vlc_credential_init(&crd, &crd_url);
>> >>>>
>> >>>> sys->manager = vlc_http_mgr_create(obj, jar);
>> >>>> if (sys->manager == NULL)
>> >>>> + {
>> >>>> + free(url_fixup);
>> >>>> goto error;
>> >>>> + }
>> >>>>
>> >>>> char *ua = var_InheritString(obj, "http-user-agent");
>> >>>> char *referer = var_InheritString(obj, "http-referrer");
>> >>>> bool live = var_InheritBool(obj, "http-continuous");
>> >>>>
>> >>>> sys->resource = (live ? vlc_http_live_create :
>vlc_http_file_create)(
>> >>>> - sys->manager, access->psz_url, ua, referer);
>> >>>> + sys->manager, url_fixup, ua, referer);
>> >>>> + free(url_fixup);
>> >>>> free(referer);
>> >>>> free(ua);
>> >>>
>> >>> --
>> >>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté.
>> >>> _______________________________________________
>> >>> vlc-devel mailing list
>> >>> To unsubscribe or modify your subscription options:
>> >>> https://mailman.videolan.org/listinfo/vlc-devel
>> >>
>> >
>> > --
>> > Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté.
>> > _______________________________________________
>> > vlc-devel mailing list
>> > To unsubscribe or modify your subscription options:
>> > https://mailman.videolan.org/listinfo/vlc-devel
>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel
--
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20201009/37d7607e/attachment.html>
More information about the vlc-devel
mailing list