Last-Order / Minyami

A lovely video downloader for HLS videos
GNU General Public License v3.0
561 stars 37 forks source link

Simplify URL joiner (and fix #71) #72

Closed fireattack closed 3 years ago

fireattack commented 3 years ago

No need to reinvent the wheel

Last-Order commented 3 years ago

Thank you.

Last-Order commented 3 years ago

landed in 4.2.2

climba03003 commented 3 years ago

After updated for 4.2.2. It break when I use the full path without host.

fireattack commented 3 years ago

Any example? Not sure what you mean by "without host".

URL obj should handle full path just fine:

new URL("https://test2.com/a.ts", "https://anotherdomain.com").href
>>> "https://test2.com/a.ts"
climba03003 commented 3 years ago

I use it to download a custom file. ./1.m3u8 or simply 1.m3u8. Which cannot be parsed by new URL

climba03003 commented 3 years ago

host passed in is not always a valid URL.

For example inside the common parser. https://github.com/Last-Order/Minyami/blob/master/src/core/parsers/common.ts#L12

m3u8Url can be a file path (both absolute or relative) which cannot be parsed by new URL. image

Basically, this fix is breaking the usage for local file. https://github.com/Last-Order/Minyami/blob/55a949bd874470de894869220b06c094f4997b44/src/utils/m3u8.ts#L48-L52

fireattack commented 3 years ago

Adding back

        if (path.startsWith("http")) {
            return path;

will restore the old behavior.

(If you look at it, it only really works with local m3u8 file + remote absolute URLs as resources; any other combination would break either way. But I guess that's enough. No one is using Minyami to process local .ts I guess.)

climba03003 commented 3 years ago

Adding back

        if (path.startsWith("http")) {
            return path;

will restore the old behavior.

(If you look at it, it only really works with local m3u8 file + remote absolute URLs as resources; any other combination would break either way. But I guess that's enough. No one is using Minyami to process local .ts I guess.)

Not sure for the local .ts part, but the .key can be local file. I have written two example which is supported before and will break in this PR in #78.