Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mega plugin ignores download path, saves files to root of C: drive #159

Closed
fake-name opened this issue Jan 22, 2023 · 10 comments
Closed

Mega plugin ignores download path, saves files to root of C: drive #159

fake-name opened this issue Jan 22, 2023 · 10 comments
Labels
bug Something isn't working

Comments

@fake-name
Copy link

fake-name commented Jan 22, 2023

Effectively the title.

PatreonDownloader.App.exe --version
Patreon Downloader 23.0.0.0

Host is W10 22H2.

I'm running this in a VM where I have very little free space in the root drive (<5 GB):

PatreonDownloader.App.exe --url https://www.patreon.com/<artist>/posts --descriptions --embeds --campaign-images --json --log-level Trace --download-directory O:\pattest\<artist> || exit /b

The Mega downloader seems to be using a creating/using cache directory named C:\V1hgz_<Mega Folder Name> where it's either saving content, or saving cache things, and it pretty quickly runs the host out of disk space and starts failing in interesting ways.

Off topic: When mega issues a 509 Bandwidth exceeded error, the downloader just winds up doing 10 retries for every mega item, which is kind of annoying. It'd be neat if it were smart enough to give up on mega files for a while when it hits a 509, but it should at least stop doing pointless retries.

@fake-name
Copy link
Author

fake-name commented Jan 22, 2023

Ok, I think something is eating the download parameter that's getting passed to the plugin, and everything else is a result of that issue. I'd guess it's something handling different drives being cranky.

Skimming the code, I don't see how, though.

Strangely enough, normal content (e.g. non-plugin downloads) get saved to the correct location (in my case, the O:/ drive), so it's not a categorical issue.

@AlexCSDev
Copy link
Owner

AlexCSDev commented Jan 22, 2023

While I appreciate all bug reports, I ask everyone to refrain from trying to teach me how things are supposed to be done without completely understanding how the application is designed and supposed to work. Thank you.

In this case it's clearly not supposed to download into the root of C: drive. Make sure you are running latest version, I cannot replicate this on release 23.

The suggestion for 509 code handling has been logged as AlexCSDev/UniversalDownloaderPlatform#14

@AlexCSDev
Copy link
Owner

Ok, since it's pretty late in the night I have actually missed the fact that you already running latest version, let me test a few things and I'll get back to you.

@fake-name
Copy link
Author

fake-name commented Jan 22, 2023

While I appreciate all bug reports, I ask everyone to refrain from trying to teach me how things are supposed to be done without completely understanding how the application is designed and supposed to work. Thank you.

Sorry, I get snarky when I'm irritated.

I wrote a bunch of that, and then dug about a bunch and realized there wasn't anything that would try to create a temp directory, and it was probably a path truncation/drive weirdness issue. I should have removed a bunch of comments at that point.

Anyways, I have VS on the system in question (it's why the disk space is so low!) so I can probably do better diagnostics once the batch download I'm running right now is complete (it might be a few days).

The suggestion for 509 code handling has been logged as AlexCSDev/UniversalDownloaderPlatform#14

Sounds good.

@AlexCSDev
Copy link
Owner

AlexCSDev commented Jan 22, 2023

Unfortunately I can't replicate it on my side no matter what I do. Unless other people report this issue or you'll be able to debug it yourself all I can guess is that you either have leftover files from older builds or some application files have managed to cache somehow.

SHA-256 hash for plugins\UniversalDownloaderPlatform.MegaDownloader.dll is supposed to be: 727B34ACE5152F668878505F502F8E86BD73C13FF06BB2C9A51E5E006A7963E9

@AlexCSDev AlexCSDev added question Further information is requested can't repro Can't reproduce labels Jan 22, 2023
@fake-name
Copy link
Author

Unfortunately I can't replicate it on my side no matter what I do. Unless other people report this issue or you'll be able to debug it yourself all I can guess is that you either have leftover files from older builds or some application files have managed to cache somehow.

I'm not actually running my own build currently, just a folder where I unzipped the latest release.

I'll stick print statements about and poke it when I can.

@AlexCSDev
Copy link
Owner

AlexCSDev commented Jan 22, 2023

Sounds good. This code is being used across 3 different applications and I don't see any issues with it on my side in any of those applications, so this is a really strange situation.

@emerladCoder
Copy link

emerladCoder commented Jan 22, 2023

I'm having the same/similar issue. Mega and Google Drive Plugins are downloading to the Root of the drive the application is running from (I have tested that by moving the application folder to different drives and it always downloads in root of the drive the application is running from, even if the download folder is in another drive)

All other files download to correct location (post, media, attachments, description, json, embed.txt, cover, avatar, etc.) Its only the plugin files and folders that are somehow losing the download directory.

I am also running the version 23. This issue does not occur in version 22 (but version 22 has the attachment naming issue bug that's more annoying in my opinion).

In the log the download path just starts with a "\"

[UniversalDownloaderPlatform.MegaDownloader.MegaDownloader] [MEGA] Downloading <filename> to \<id>_<filename>

[UniversalDownloaderPlatform.GoogleDriveDownloader.Plugin] Retrieved id: <full google id>, download path: \<id>_

Instead of like in version 22 where it has the download directory

[UniversalDownloaderPlatform.MegaDownloader.MegaDownloader] [MEGA] Downloading <filename> to <download-directory>\<id>_<filename>

Edit:
I started from fresh download and tested on version 22 and 23 for above results. So I don't think its any cache issue or anything like that.

Edit2:
I think I may have found the source of the issue?

I was debugging the Mega plugin and found that crawledUrl.DownloadPath is equal to "\" (inside DownloadUrlAsync function in MegaDownloader.cs) thus Path.Combine(downloadPath, crawledUrl.DownloadPath, ...) returns starting with second parameter that starts with "\" and is stripping first parameter.

Similarly inside Plugin.js line 77 for google plugin there is Path.Combine(_settings.DownloadDirectory, crawledUrl.DownloadPath) which is being truncated to just the second parameter as well for the same reason.

Starting with "\" indicates an absolute path, thus any preceding parts are dropped by the Path.Combine function from my testing.

In Patreon URL processor we are setting the crawledUrl.Downloadpath in Line 181 in PatreonCrawledUrlProcessor.cs
crawledUrl.DownloadPath = !crawledUrl.IsProcessedByPlugin ? Path.Combine(downloadDirectory, filename) : downloadDirectory + Path.DirectorySeparatorChar;

Plugin URLs will always use downloadDirectory + Path.DirectorySeparatorChar from the Ternary Operator which will always equal just a "\" for plugin urls in the above case since downloadDirectory is set to an empty string "" just above this.

So either we should pass the full path for plugin case so that when Path.Combine drops the first parameter it will still have full path.
crawledUrl.DownloadPath = !crawledUrl.IsProcessedByPlugin ? Path.Combine(downloadDirectory, filename) : _patreonDownloaderSettings.DownloadDirectory + Path.DirectorySeparatorChar;

Or we shouldn't be combining downloadPath and crawledUrl.DownloadPath in the Path.Combine in the plugins as they represent the same thing?

Edit3:
I suppose if you use --use-sub-directories the downloadDirectory will not be blank thus downloadDirectory + Path.DirectorySeparatorChar would not start with a "\" and you wouldn't get this issue. So ideal solution/fix should ensure both options work.

Maybe something like:
crawledUrl.DownloadPath = !crawledUrl.IsProcessedByPlugin ? Path.Combine(downloadDirectory, filename) : (downloadDirectory == "" ? _patreonDownloaderSettings.DownloadDirectory : downloadDirectory);

Pass the full path if downloadDirectory is still blank because --use-sub-directories is not set else pass the downloadDirectory value. It shouldn't be necessary to append the Path.DirectorySeparatorChar as that will be added automatically by the Path.Combine function. I think the only reason you have Path.DirectorySeparatorChar added is so that the value will not be blank and thus fail the IsNullOrWhiteSpace check that would prevent the item from downloading (alternately just return downloadDirectory and remove the IsNullOrWhiteSpace check if the entry IsProcessedByPlugin thus allowing the blank value since we don't know the file name at this point).

@AlexCSDev
Copy link
Owner

AlexCSDev commented Jan 22, 2023

@emerladCoder Thank you for your detailed look into the problem. This is actually an issue which slipped through while I was doing refactoring. Since DownloadPath is supposed to represent a directory relative to the download path it should be completely empty when --use-sub-directories is not used and the url was processed by the plugin. The absolute path is supposed to be built on the plugin's side. Will fix that asap.

AlexCSDev added a commit that referenced this issue Jan 22, 2023
… the drive when --use-sub-directories is not enabled (#159)
@AlexCSDev
Copy link
Owner

Released new version addressing this issue: https://github.com/AlexCSDev/PatreonDownloader/releases/tag/release_24

@AlexCSDev AlexCSDev added bug Something isn't working and removed question Further information is requested can't repro Can't reproduce labels Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants