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

Fix acknowledge abuse download of unflagged files #410

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

felixbuenemann
Copy link
Collaborator

Currently, when the --acknowledge-abuse flag is given, plexdrive can only download abused files, files not flagged for abuse fail to download with the error "invalidAbuseAcknowledoment".

This PR tries to fix that by only adding the acknowledgeAbuse=true parameter to the download URL, if the --acknowledge-abuse was given and the download request failed with the "cannotDownloadAbusiveFile" error.

The drawback is that we do twice as many download requests for abused files, but a better solution would require to cache the abuse state of a file in the drive object metadata inside cache.bolt, which would require a lot more code changes.

Unrelated to the fix I've also moved the global acknowledgeAbuse option flag to be passed directly to the Downloader instance, instead of repeating it in each Request struct.

It is a global flag, so no need to pass it as part of the request.
This fixes download of files not flagged for abuse, if the
--acknowledge-abuse option is used.
@felixbuenemann
Copy link
Collaborator Author

Do not merge yet, I'm waiting on feedback in the discussion of this issue in #402. I could only test the case for files which aren't flagged for abuse so far, since I don't have any flagged files.

Copy link

@Moodkiller Moodkiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also confirm that that running with --acknowledge-abuse allows non flagged files to be downloaded now. No flagged files here either.

Okay just had a brain wave and tested on a file that is a false positive (nicehashes quick-miner)
image

options: -o allow_other -v 3 -o direct_io --acknowledge-abuse --chunk-check-threads=2 --chunk-load-ahead=3 --chunk-load-threads=2 --chunk-size=10M --max-chunks=64 --refresh-interval=5m

Would it also be possible to add a logic that eventually skips / fails the download all together? Use case: The user hasn't specified --acknowledge-abuse but the file is flagged on drive.

@felixbuenemann
Copy link
Collaborator Author

@Moodkiller Not sure why the download of the flagged file doesn't work for you, even though --acknowledge-abuse is specified. Does it work with plexdrive 5.2.1 and --acknowledge-abuse?

I think you also need to be the owner of the file, to be able to download it.

Would it also be possible to add a logic that eventually skips / fails the download all together? Use case: The user hasn't specified --acknowledge-abuse but the file is flagged on drive.

If the download fails you get the "Could not read object ... / StatusCode: 403" error, but that is for each chunk. Plexdrive doesn't remember that something failed and that's not easy to change, since we would have to add some kind of blacklist of files that should instantly fail.

@Moodkiller
Copy link

@Moodkiller Not sure why the download of the flagged file doesn't work for you, even though --acknowledge-abuse is specified. Does it work with plexdrive 5.2.1 and --acknowledge-abuse?

Sorry for the delay! That was an excellent test. Yes, I can download the file with plexdrive-5.2.1 --acknowledge-abuse but not with the patched version. Interesting indeed...My guess is that the flag is not getting passed to the download url WHEN the file is flagged? Is there a way I could output the downloadurl to check? Im using -v 3 fwiw.

If the download fails you get the "Could not read object ... / StatusCode: 403" error, but that is for each chunk. Plexdrive doesn't remember that something failed and that's not easy to change, since we would have to add some kind of blacklist of files that should instantly fail.

I see I see... I dont expect it to be a massive problem to be fair. Usually entire drives get nuked before a single file gets flagged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants