caprica / vlcj-natives

Native bindings to LibVLC used by the vlcj project
14 stars 6 forks source link

missing check for file:/// on making a new File #6

Open SrRapero720 opened 5 months ago

SrRapero720 commented 5 months ago

When the mrl is converted to ASCII. it first converts the uri into a File to then check if starts with file://

problem starts when you already sends the mrl with the file:// protocol. Making a new File will ends in a nested path such as file:///C:\path\of\the\process\C:\path\of\the\process\config\myvideo.mp4

https://github.com/caprica/vlcj-natives/blob/72e800fd461bf63a7ea4aa0008fdfa7f71e14c0d/src/main/java/uk/co/caprica/vlcj/binding/support/strings/NativeUri.java#L120-L129

the fix for that is before convert to file, check if the uri starts with file:/// (according with the format of new URL()), so it can be safety converted into a file to then convert it into a ASCII string

if (value.startsWith("file:///")) value = value.substring("file:///".size() - 1)
caprica commented 5 months ago

I am not so sure about this to be honest. There is quite a bit of history around this functionality, at the moment I can't recall all of the details unfortunately, and it's been like this a long time.

I'll look into it.

SrRapero720 commented 4 months ago

doing some look into it it only do the file:/// check when it contains unicode charachers, but doesn't do the check when it doesn't basically if i pass a path like A:\developer-code\modding\watermedia\src\main\resources\videolan it will consider it as a valid uri with no unicode, passing the protocol check and of couse this will not work

adding (another) basic check also fixes it too. paths on java really confuses me, considering exists like 4 ways to manage these (URL, URI, File and Path). but certainly this can be addressed in a more cleaner way than fill the code with a lot of file:/// checks

for me works as a workarround image

caprica commented 4 months ago

if you pass "A:\whatever" then it isn't a URL, it's a local file.

caprica commented 4 months ago

There was already an issue I opened in the vlcj project itself to determine if this code was still required at all in fact.

I have a nagging doubt about this code, it has been in vlcj for a long time, for some specific reasons.

Hopefully there's something in the commit log about this file which will prompt me to remember.

caprica commented 4 months ago

https://github.com/caprica/vlcj/issues/1142

caprica commented 4 months ago

For context: https://github.com/caprica/vlcj/issues/415 https://github.com/caprica/vlcj/issues/645

SrRapero720 commented 4 months ago

if you pass "A:\whatever" then it isn't a URL, it's a local file.

Indeed, but local files in a file:/// are considered URLs

Anyway, seeing MRL doesn't longer need a special encoding. The best option is change the MRL type from String to java.lang.URL. which also help to ensure whatever you're passing is a valid URL (just internally changing it toString())

for file protocol is enough doif (result.startsWidth("file:/")) result = result.replace("file:/", "file:///");

caprica commented 4 months ago

OK, but really if you do have a local file, you're not supposed to pass it as a URL.

LibVLC has two dedicated native functions for loading a local file vs an "MRL". An MRL is like a URL but I'm not sure it's 100% interchangeable.

It's not Java's URL class that's doing the work here, it has to be serialised a string, and passed to the native function as that string.

This is why I'm a bit wary of this problem, as it comes down to how VLC handles MRLs.

SrRapero720 commented 4 months ago

passing the file path as an URL works fine when I pass the file path directly it simply just don't work No error is emitted

SrRapero720 commented 2 months ago

Regardless my suggestion of switch encodeUri and convert from a string to java.io.File and java.lang.URL after some time experimenting with this, URL doesn't have support for some protocols that VLC supports like srt://. the idea of use URL is to validate the url syntax by the dependent side before send it to VLCJ, so internally only have to convert into a string and use libvlc_media_new_location. and for File and Path directly call libvlc_media_new_path

It's not Java's URL class that's doing the work here, it has to be serialised a string, and passed to the native function as that string.

Yes, neither URI or File are doing nothing except "validate if is a path or a url".

caprica commented 2 months ago

Could you please maybe summarise the current difficulty here, i.e. the current state of this issue/request from your point of view?

For wider context, I am struggling to remember the full history of why this code is in vlcj since it has likely been here for over 10 years in some form or another.

I think part of it is to do with handling UNICODE character conversions, which was a particular problem with vlcj on older versions of Java, and maybe VLC too.

SrRapero720 commented 2 months ago

I was a little disconnected so i lost the track of what was happening inside. i have to re-study all the code

Summary

My issue is based on the process to parse a file URL. URL#toString() converts an URL like file:///c:/path/to/file.mp4 into file:/c:/path/to/file. That url is invalid if you try to place it back on a URL like new URL(new URL("file:///c:/path/to/file").toString()) it throws an exception

Also VLC rejects that Uri syntax/protocol (file:/c:/) so not sure why Java returns that cursed Uri.

What i did is convert the mrl string to always have the file:/// so i can avoid do any odd convertion. Problem is the method encoder, which converts the file:///c:/path/to/file.mp4 into a "nested path" of itself like this file:///C:\path\of\the\process\C:\path\of\the\process\config\myvideo.mp4 as i mentioned before

This was caused by this code: https://github.com/caprica/vlcj-natives/blob/72e800fd461bf63a7ea4aa0008fdfa7f71e14c0d/src/main/java/uk/co/caprica/vlcj/binding/support/strings/NativeUri.java#L120-L129

that code basically wraps the URL as a file, and because is not a normalized path string it gets attached into process dir, making the path looks like file:///C:\path\of\the\process\C:\path\of\the\process\config\myvideo.mp4

ASSUMPTIONS (nothing checked)

I honestly removed any ASCII encoding and it still works, accepts few odd urls with unicode characters. Thats because in the old days ASCII was the standard, considering how often was required tell to gradle, java and few text editors to manually use UTF-8. I assume VLC 2 was using ASCII as default but got changed on VLC 3(?

In any case i was working with that on watermedia for a long time and there wasn't reported issues about URL's getting failed to load by VLC due to bad MRL sources

IMPORTANT SPOTS

My suggestions

One of the fixes i did on the silly fork i made was use java.io.File for local files and java.lang.URI for URLs. that doesn't do any encoding to ASCII or UTF-8, it simply serves to idenfity the type of the MRL. if was a file or a "online source". Internally VLCJ doesn't have to adivinate what type of MRL is, instead you're making the dependent dev to serve the proper MRL type.

Extra

URI also forces dev to encode the URL to mitigate "non-valid" chars image

VLC supports both types of file MRL, as URL or as a File path, URL Encoded or not image image