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

Handle downloading files.redgifs.com URLs #36

Closed
wants to merge 1 commit into from

Conversation

flagarby
Copy link

@flagarby flagarby commented Sep 3, 2024

Some download URLs are now redirecting to files.redgifs.com direct downloads. Example traceback:

Traceback (most recent call last):
  ....
  File "/home/flagarby/scripts/link_dropper.py", line 158, in drop_redgifs
    rg_api.download(gif.urls.hd, f)
  File "/home/flagarby/.pyenv/versions/scripts/lib/python3.10/site-packages/redgifs/api.py", line 394, in download
    return self.http.download(url, fp)
  File "/home/flagarby/.pyenv/versions/scripts/lib/python3.10/site-packages/redgifs/http.py", line 263, in download
    raise TypeError(f'"{strip_ip(str_url)}" is an invalid RedGifs URL.')
TypeError: "https://files.redgifs.com/SomeRandomIdentifier.mp4" is an invalid RedGifs URL.

@scrazzz
Copy link
Owner

scrazzz commented Sep 3, 2024

Hello, first of all, thank you for reporting this issue and opening a PR to fix it.

Now about this PR. I have a suggestion. I think we should stop replying on the subdomain (thumbs, api, files) because it could change anytime. Rather validating the URL, I should be checking the response's Content-Type since that would be more reliable. If the Content-Type is not video/mp4 then I can provide a more meaningful error. What do you think?

@flagarby
Copy link
Author

flagarby commented Sep 4, 2024

Very likely, but I haven't tested enough to know how reliable that would be. I was just trying to fit your existing style for this PR before I forgot to submit my local changes.

The URL matching could probably be simplified to this:

diff --git a/redgifs/http.py b/redgifs/http.py
index eb983e5..5185e6e 100644
--- a/redgifs/http.py
+++ b/redgifs/http.py
@@ -249,18 +249,9 @@ class HTTP:
                     return f.write(data)

         # If it's a direct URL
-        if all([x in str(yarl_url.host) for x in ['thumbs', 'redgifs']]):
-            match = re.match(REDGIFS_THUMBS_RE, str_url)
-            if match:
-                return (dl(str_url))
-            raise TypeError(f'"{strip_ip(str_url)}" is an invalid RedGifs URL.')
-
-        # If it's a 'files' URL
-        if all([x in str(yarl_url.host) for x in ['files', 'redgifs']]):
-            match = re.match(REDGIFS_FILES_RE, str_url)
-            if match:
+        for pattern in (REDGIFS_FILES_RE, REDGIFS_THUMBS_RE):
+            if re.match(pattern, str_url):
                 return (dl(str_url))
-            raise TypeError(f'"{strip_ip(str_url)}" is an invalid RedGifs URL.')

         # If it's a 'watch' URL
         if 'watch' in yarl_url.path:

...and then a Content-Type check could be added as a preliminary (or folllowup) check. That could allow for a fallback scenario until you're more confident with the logic change.

@scrazzz
Copy link
Owner

scrazzz commented Sep 4, 2024

I was thinking of not using regex for this. I don't even know why I used regex here in the first place because the host (redgifs.com) can be validated using yarl.

Anyways, I've pushed an update right now and from my testing the video gets download and got no errors:

from redgifs import API

api = API().login()
result = api.get_trending_gifs()
url = result[2].urls.sd
api.download(url, 'video.mp4')

@scrazzz
Copy link
Owner

scrazzz commented Sep 5, 2024

Hi, I haven't gotten a response and I noticed that this is (possibly) an alt account you've created for this PR. I've gone ahead and fixed this issue along with other minor changes: f8f7e90

Thanks for opening this PR to address the error asap.

@scrazzz scrazzz closed this Sep 5, 2024
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.

2 participants