Release YOUTUBE Downloader v2.9

[tormyvancool]

1.0 2024-26-10
# First Release
1.1 2024-26-10
+ Processes Notifications
- /Video/
+ /Videos/
1.2 2024-26-10
- --merge-output-format mp4
+ -S vcodec:h264,res,acodec:aac
1.3 2024-26-10
- 10
+ 2
1.4 2024-26-10
- 2
+ 5
1.5 2024-26-10
- 5
+ 1
# Unified Update
1.6 2024-26-10 - 1
+ 2
+ Version
1.7 2024-27-10
- 'start "" "' from all O.S.s
+ 'start "UPDATE & DOWNLOAD" "' Win
1.8 2024-27-10
- GGGGG = ''
- 1
+ Start = '"'
+ 2
1.9 2024-27-10
+ Check saved project
- 1
+ 2
2.0 2024-27-10
- "chmod +x " .. MainPath
+ 'chmod +x "' .. MainPath .. '"'
# Ordered Variables
- 2
+ 1
+ Apple Trial
2.3 2024-27-10
# Linux execution correction
+ Credits
# 2.1 and 2.2 just trials due issues with Linux and Apple
2.31 2024-28-10
# Binaries directly form the source
2.32 2024-28-10
- yt-dlp
+ yt-dlp_linux
2.4 2024-29-10
# Adjusted header style for production
2.5 2024-11-04
- Various
+ VideoPath = 'Video'
2.6 2024-11-05
+ check for temrination of temporary file upfrotn import the video
2.7 2024-11-05
- Check Routine
2.8 2024-11-06
+ Detects Nework Interruptions during download
+ Removes leftovers
+ URLs as filename: forbidden
+ Limitation to only alphanumerical characters
2.9 2024-11-06
+ Check IfFileExists: Overwrite, Newname, Exit

[cfillion]

It's an improvement over last time however there are still significant design flaws that I can't accept on a default repository. Most notably the complete absence of any protection against injection which is unchanged from the previous PR.

In the current state I think it would be better released on your own repository.

[tormyvancool]

It's an improvement over last time however there are still significant design flaws that I can't accept on a default repository. Most notably the complete absence of any protection against injection which is unchanged from the previous PR.

In the current state I think it would be better released on your own repository.

I went through all what highlighted and committed the solutions.

[tormyvancool]

Ok I think i've done with this script. All what we discussed and also something more, was done and the "something more" was added.
So in case something goes wrong, the script shouldn't freeze Reaper indefinitely, and informs the operator stopping itself and let Reaper free of working.

[cfillion]

Going to reject this for now. Thanks for applying some of the smaller suggestions, however the major flaws pointed out in both this and the previous PRs are still not addressed. Shell command execution requires more care than most scripts and must either be done right or not attempted.

[tormyvancool]

Shell command execution requires more care than most scripts and must either be done right or not attempted.

Shell commands are for what it should be done the way I expect. The function you suggested me: doesn't and it creates more issues than benefits.

I didn't say that? There is no reason to check whether ReaPack has installed yt-dlp. It either does or the entire package is not > installed at all & the user is informed of the error.

I was talking about the broken & redundant chmod which is still not removed.

That's why I asked for the confirmation of what I understood. You came today. I asked 3 days ago. And I added "ok no feedback = free interpretation :-)" so meanwhile I worked on what I understood.

The redundant chmod +x is required since during the installation of the package it seems there is not way to guarantee the chmod +x as required to the downloaded binary. And this is was clearly issued during the testing of the previous versions. Indeed yt-dlp was downloaded without that flag (it turned out to me only read mode). Solution: run that command to be sure is +x

If I ask to the O.S. to "chmod +x" when the file already is in "x" nothing happens. It survives on this and it doesn't require more than some microsecond.

[tormyvancool]

NOTE the fact you closed this PR, obliges me to pass again that uncomfortable form.It requires more effort there tan to remove a redundancy on os.execute() with the issues on downloading only the 64 bits etc ...

[cfillion]

Shell commands are for what it should be done the way I expect. The function you suggested me: doesn't and it creates more issues than benefits.

os.execute is fine if the input is properly sanitized (not trivial on Windows, not too bad on Unix-likes). ExecProcess is easier in this aspect, but of course it comes with limitations (no escape character = worse than os.execute for compatibility) and can be tricky to use without understanding it.

That's why I asked for the confirmation of what I understood. You came today. I asked 3 days ago.

...are you really complaining because I didn't reply within the 2/6 hours between your "I need your feedback to work on this" and "ok no feedback = free interpretation" / "Ok super. Committed." replies?

The redundant chmod +x is required

Two chmod are redundant. One is enough. Looks like you tried to fix that in #1461 by removing one of them. Good but the one that's left is not going to work without being provided the correct path to the file.

NOTE the fact you closed this PR, obliges me to pass [...]

You can re-create a PR without going through the form again (same git branch). But please go ahead and release it in your own repository instead. I'm sure the script is useful and mostly works but I can't accept it here (where it's going to be shipped to all users) unless the various packaging and core functionality issues are fully addressed.

• Hardened against command injection
• chmod not broken on macOS – since you're testing on Linux: macOS is pretty much the same yet your code is different for some reason
• yt-dlp pinned to a specific version (when an update –yours or yt-dlp– breaks something, users need to trust ReaPack's downgrade feature to undo the update without breaking a sweat)
• No multi-version changelog

While at it (optional, out of scope for a packaging review) the code quality could be improved. There's unused leftovers, inconsistencies and typos (such "Downlaod" and "filname")...

[tormyvancool]

Shell commands are for what it should be done the way I expect. The function you suggested me: doesn't and it creates more issues than benefits.

os.execute is fine if the input is properly sanitized (not trivial on Windows, not too bad on Unix-likes). ExecProcess is easier in this aspect, but of course it comes with limitations (no escape character = worse than os.execute for compatibility) and can be tricky to use without understanding it.

How to sanitize it more? I have not clue. Or I wasn't make mistakes.
If you notice that os.execute() base don OSes in the beginning are just paths taken by REPAER's APIs.
The same for the variables "Update" and "Video" used into the os.execute() almost at the end of the script.

The input fields (the only oen way I see a potential injection)

• the first one verifies is an URL and it must be valid
• the second one has limited inputs: only alphanumerically characters.

...are you really complaining because I didn't reply within the 2/6 hours between your "I need your feedback to work on this" and "ok no feedback = free interpretation" / "Ok super. Committed." replies?

In 3 days (not 6 ours) I was working on it trying also to better understand what you were saying. Do not have feedback I just applied what I did understand. It's not an accusation but it clears out how I was working.
however the control I have pt in place does its job. So ppl will install the script via ReaPack system avoiding to download just the script, copy and paste it and use it, missing the important settings.

Two chmod are redundant. One is enough. Looks like you tried to fix that in #1461 by removing one of them. Good but the one that's left is not going to work without being provided the correct path to the file.

Yes I spotted it after this. That's why I corrected it and opened the PR that you closed now.

You can re-create a PR without going through the form again (same git branch). But please go ahead and release it in your own repository instead. I'm sure the script is useful and mostly works but I can't accept it here (where it's going to be shipped to all users) unless the various packaging and core functionality issues are fully addressed.

That's why I started to ask for HELP everywhere (for the os.execute()). But I don't get the hints I do need to sanitize that damn os.execute()
I'm not born with the knowledge embedded or I wasn't here neither.

Example? tons; For instance how can I reopen a PR by updating the code avodigin that form and avoiding to open a fork?
I cannot edit the code pasting the new one now that's closed.

• Hardened against command injection

This where I'm still asking for but not answers that really helps me not only to understand but to avoid future mistakes.

• chmod not broken on macOS – since you're testing on Linux: macOS is pretty much the same yet your code is different for some reason

In the new code (3.0) I removed it.

• yt-dlp pinned to a specific version (when an update –yours or yt-dlp– breaks something, users need to trust ReaPack's downgrade feature to undo the update without breaking a sweat)

yt-dlp is not released if it breaks everything. yes can have bugs like Reaper has bugs. But it's not breaking anything.

• No multi-version changelog

I explained you that the detailed changelog served to keep history on what it was done and ReaPack has not that history since from 1.0 to 30 was published on my repository. The form on RePack site, is a text box that enables to paste the whole history.

While at it (optional, out of scope for a packaging review) the code quality could be improved. There's unused leftovers, inconsistencies and typos (such "Downlaod" and "filname")...

[cfillion]

How to sanitize it more? I have not clue.

• https://www.gnu.org/software/bash/manual/bash.html#Quoting
• https://daviddeley.com/autohotkey/parameters/parameters.htm#WINCRULESDOC
• https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/

That's why I corrected it and opened the PR that you closed now.

You removed the chmod that was working and kept the one that didn't.

yt-dlp is not released if it breaks everything. yes can have bugs like Reaper has bugs. But it's not breaking anything.

When something break, which also includes undesired changes of any kind, users need to be able to easily and reliably downgrade.

https://xkcd.com/1172/

how can I reopen a PR

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

I explained you that the detailed changelog served to keep history

The purpose of changelogs is to inform/warn users of changes between versions when they install or update. There is no purpose for changelogs before the first ReaPack release.

Keeping full development history is git's purpose.

The form on RePack site, is a text box that enables to paste the whole history.

It says immediately below it: "Changelog for the current version only".

[tormyvancool]

How to sanitize it more? I have not clue.

• https://www.gnu.org/software/bash/manual/bash.html#Quoting
• https://daviddeley.com/autohotkey/parameters/parameters.htm#WINCRULESDOC
• https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/

That's why I corrected it and opened the PR that you closed now.

You removed the chmod that was working and kept the one that didn't.

oops restored and removed the one inside the os.execute()

However about the quoting: what's need to be quoted? Since practically everything is generated by the REAPER's APIs
Or do you mean to avoid the double -quotes? For instance

from MainPath = '"' .. CallPath .. dlpLnx .. '"'
to MainPath = "'" .. CallPath .. dlpLnx .. "'" ?

On the pull request: once it's closed I have this

I can't edit any longer.

An this
"In the "Branch" menu, choose the branch that contains your commits." is not valid once the PR is closed.

[cfillion]

restored and removed the one inside the os.execute()

👍 (Alternatively could have fixed the os.execute one by giving it the correct path.)

However about the quoting: what's need to be quoted?

(Linux&macOS) Single quotes are easier but the possibility of those variables containing single quotes and backslashes themselves has to be handled.

once it's closed I have this

The grayed out 'edit file' button in the screenshot is a shortcut to a similar button on your fork.

Good luck with your script.

[tormyvancool]

restored and removed the one inside the os.execute()

👍 (Alternatively could have fixed the os.execute one by giving it the correct path.)

However about the quoting: what's need to be quoted?

(Linux&macOS) Single quotes are easier but the possibility of those variables containing single quotes and backslashes themselves has to be handled.

once it's closed I have this

The grayed out 'edit file' button in the screenshot is a shortcut to a similar button on your fork.

Good luck with your script.

The correct path in the last version was given by the API by Reaper. Indeed I changed the way these variables were generated.

About the quotes: besides the only few quotes I put, all the other parameters (like the path and filenames) are generated by Reaper's APIs. I think these APIs are already managing this issue and sanitizing the content.