jeckman / YouTube-Downloader

PHP script for downloading videos from youtube; also parsing youtube feed into RSS enclosures for podcatchers
GNU General Public License v2.0
895 stars 474 forks source link

SignatureDecipher needs a rewrite #300

Closed Art4 closed 6 years ago

Art4 commented 6 years ago

The YouTube-Downloader\Provider\Youtube\SignatureDecipher.php needs a whole rewrite with better encapsulation, more tests and an improved error handling.

These are issues that can't easily fix because of the messy SignatureDecipher:

StefansArya commented 6 years ago

Interesting.. You said it messy but you don't know how to remake it and put copyright notice on the modified script that was originally written by me..

Art4 commented 6 years ago

@StefansArya I'm deeply sorry. :slightly_frowning_face: It was never my intention to criticize someone. This issue was only supposed to be a point for the Refactoring project, see https://github.com/jeckman/YouTube-Downloader/projects/1

When I said "messy" than I mean the big static methods of the class and the dependencies (call to curl and die) that makes it hard to write tests or fix bugs, but not that I don't know how to refactore it. I want to apologize if I used the wrong words to describe the purpose of the issue. :frowning_man: Please keep in mind that English is not my native language.

About the copyright notice: This project is licensed under GPL and that's why I decided to invest time for this project. I'm a huge supporter of FOSS and I want to give something back to the open source community. Also I want to improve my PHP skills.

The reasons for putting a the license note into every file are:

  1. it is recommended in the GPL-2.0 license file:

    To do so, attach the following notices to the program. It is safest to attach them to the start of each source file to most effectively convey the exclusion of warranty;

  2. I want to make it a little harder for someone to ignore the license and sell it as proprietary software, like in #134.

I also want to note that I never put my own name into the license note because most of my work was based on the work of others. I simply put the name of @jeckman as project initiator into every file and asked for feedback for this decision in #261. It's not my intention to get "fame" or something. I just want to help building a nice and useful tool. But I have respect for your work for this project especially for the decipher class and it would be fair to keep your name into this files. I can add it if you wish it.

As I said I didnt' want to displease anyone and I want to apologize again for my words. I hope you will accept my apology and I'm looking forward to make this project better together as a community. :muscle:

StefansArya commented 6 years ago

Well it's okay.. Actually I'm not a big fan of framework as it slow down my machine when I have so many visitor.. I would prefer develop a simple PHP class that focused on performance and security..

I also have a public library on my repo, maybe you could learn from there..

ewwink commented 6 years ago

the problem is explode in getPlayerInfoByVideoId

$playerID = explode("\/yts\/jsbin\/player-", $playerID);

sometimes it used "_" underscore ":"\/yts\/jsbin\/player_ias-vfloXMnp9\/id_ID\/base.js

Quick Fix, Replace:

$playerID = explode("\/yts\/jsbin\/player-", $playerID);

if (count($playerID) === 0) {
    throw new \Exception(sprintf(
        'Failed to retrieve player script for video id: %s',
        $videoID
    ));
}

$playerID = $playerID[1];
$playerURL = str_replace('\/', '/', explode('"', $playerID)[0]);

with

$separator = "-"; 
$playerID = explode("\/yts\/jsbin\/player-", $playerID);

if (count($playerID) < 2) {
    $playerID = explode("\/yts\/jsbin\/player_", $playerID[0]);
    $separator = "_";
}

if (count($playerID) === 0) {
    throw new \Exception(sprintf(
        'Failed to retrieve player script for video id: %s',
        $videoID
    ));
}

$playerID = $playerID[1];
$playerURL = $separator . str_replace('\/', '/', explode('"', $playerID)[0]);

Replace

public static function downloadRawPlayerScript($playerURL)
{
    return self::loadURL(
        'https://youtube.com/yts/jsbin/player-' . $playerURL
    );
}

With

public static function downloadRawPlayerScript($playerURL)
{
    return self::loadURL(
        'https://youtube.com/yts/jsbin/player' . $playerURL
    );
}
StefansArya commented 6 years ago

Yeah I know I already submitted my updated version of the signature decipher #304

jeckman commented 6 years ago

Closing this issue because we merged in #304