-
Notifications
You must be signed in to change notification settings - Fork 213
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
Re-fix Redgifs #659
Re-fix Redgifs #659
Conversation
API seems to return incorrect signature value when sending header. Other fixes seems to have worked temporarily but have stopped working so they're removed.
I replaced my local installation of the repo's current site_downloaders/redgifs.py file with the one from this pull request. https://i.redgifs.com/i/oddballfemalehousefly.jpg --> https://www.redgifs.com/watch/oddballfemalehousefly (BDFR Downloaded the HTML of page instead of images on page) (2 images here) https://www.redgifs.com/watch/willingwindyyellowbelliedmarmot (BDFR crashed upon attempting downloading from this link) (4 images here) The videos appear to be downloading perfectly fine. python -m bdfr download --log ./Log -S new -s tgirls . [ 350 successful downloads ] Traceback (most recent call last): |
I will look into that shortly, I haven't run into it yet and didn't think to test it as it looked like it should work from what I had seen in the api. Can you post the reddit ID's for those links? (6 character, likely starts with x) |
Okay, seems I pulled a stupid and only removed the headers from one of the two retrieve_url. Can you try with the new changes to make sure it works correctly for you? |
HTML was downloaded instead of image: xextbj (has .jpg file extension but actual contents is HTML) Just updated the redgifs.py file |
Fix 652 worked for me but I only tested with videos not images. Thank you Soulsuck24! I will test 659 as well. |
Looks like they're both galleries so if one works the other should too, hmmm. I'll see what I can figure out. |
To be fair I've had the HTML issue for over a year. It's not just from redgifs though, I remember getting an HTML file from xvideos & onlyfans + random websites before. Occasionally had .com extensions. It's not really a big deal. |
I did some more tests & found 24 more HTML files disguised as JPEG files, and every single one of them had links that APPEARED to be direct links to a file but were actually redirect links [2022-09-16 15:51:02,137 - bdfr.downloader - DEBUG] - Using Direct with url https://imgur.com/a/OGGGZkd.jpg The https://www.redgifs.com links appear to be broken links that redirect to a nonexistant webpage |
For now I'd say use --skip-domain=redgifs.com till it's fully solved. But, progress is being made at least. It seems the url is being stripped of the new expires, signature and for parameters before attempting to download. So just need to find out how to correct that without breaking other things. Maybe, I'm not sure if that's what it is anymore... sigh |
I thought the problem was that it's using the direct downloader instead of the site specific downloader because the link appears to be a direct link to a file but it's actually a redirect so the direct downloader is not usable but it's already been selected because of the link appearing to point directly to a file. Was I wrong? |
The command below downloads the same exact file that BDFR downloads for the reddit post with the id of xextbj |
It seems to be a contribution from both. Still working away at it. I thought I had sorted it out but it's still doing weird things. |
I just noticed that https://i.redgifs.com/i/oddballfemalehousefly.jpg is the link that is posted on the post over at xextbj |
If this doesn't work then I give up...
Okay, I do believe I've got it fixed now. It turns out it was nothing to do with the parameters. Needed to move up the Redgifs option in the download factory and then figure out why the re.match wasn't working the way that it was already (honestly no idea why it needed that change to work with those links). So this should fix the Redgifs module. I will look into the Imgur one that was posted later. I'm done with this repo for the night lol |
As far as I can tell, RedGifs downloads are working perfectly now. |
Hey thanks for all this work, I've been a bit swamped. I gotta update the tests but if they go fine I can merge it. |
No trouble, it seems like it was mostly edge cases that weren't being taken care of neatly. I'll be the first to admit I don't really run the tests in the repo so would be more likely to break them than update them properly so I'll leave that part to you. I'll move on to the imgur edge cases that Kylar posted and see if I can sort them out as well. |
Have you figured out anything about why the Imgur downloader wasn't working on some posts? |
Cover edge cases that shouldn't ever happen but probably will sometime. Also included Imgur changes to cover similar situations of malformed/redirected links.
So this should hopefully get both sites working properly and cover all the edge cases that I can think of. I wasn't able to dig up any reddit ID's for the Imgur redirects so couldn't test myself but the logic works the same in this version as it does for Redgifs so there shouldn't be any issues. |
One more thing to mention: During one of my tests yesterday I had one of the runs stall indefinitely with no indication of progress when it came across a post with a french pornhub link, but english pornhub links worked just fine Edit: I just did some tests with the latest commit & it now just says "Could not download submission xg5404: No downloader module exists for url" & moves on to the next post, which I am content with. Probably the only french pornhub link I'll ever come across anyway. |
All of the Imgur downloads are downloading as an HTML file with a .gifv extension with the latest commit |
This reverts commit 2f2b5b7.
Cover edge cases that shouldn't ever happen but probably will sometime.
regex is a scourge Since this pull was for Redgifs and that seems complete I'm going to move the imgur stuff to #662 and work on it separately, which I should have done in the first place. |
I love regex lol I'll get to work on the tests...with regex |
Hey @Soulsuck24 can you please give me push permissions so I can add my tests commit a la here. (Also for your other branch if it's the same.) |
@Serene-Arc As far as I can tell you should be able to for both. That checkbox is checked in both PR's. I unchecked and rechecked both, hopefully that fixed it if it wasn't working. Worst case you can just close both and make a new one for yourself with everything. |
Hm I will investigate |
API seems to return incorrect signature value when sending header. Other fixes seems to have worked temporarily but have stopped working so they're removed.
should fully fix #652 and #661