-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[bunkr] Intermittent ValueError on redirects #6790
Comments
I wrote a (very) dirty patch to fix it in the meantime since it only happens specifically with 301 codes, but IMO this should be handled properly by checking the value of the Location header instead of relying on the response code:
|
Thank you for the detailed error report and the reproducible example URL for bunkr. This patch will also fix it: diff --git a/gallery_dl/extractor/bunkr.py b/gallery_dl/extractor/bunkr.py
index 3e124525..7b8294cc 100644
--- a/gallery_dl/extractor/bunkr.py
+++ b/gallery_dl/extractor/bunkr.py
@@ -80,6 +80,9 @@ class BunkrAlbumExtractor(LolisafeAlbumExtractor):
# redirect
url = response.headers["Location"]
+ if url[0] == "/":
+ url = text.root_from_url(response.url) + url
+ continue
root, path = self._split(url)
if root not in CF_DOMAINS:
continue |
it still fails with albums
|
|
The issue is that there seems to be an extra 301 redirect sometimes, and the value of the Location header, and thus the
url
variable here is sometimes in the form/f/rEeTUL8MXR17A
instead ofhttps://bunkr.ph/v/rEeTUL8MXR17A
, hence causing the retrieval of the index of the character/
starting at position 8 to fail.It only happens intermittently (running the same command a couple times for the same album will eventually retrieve the right value), but it's probably better and more robust to ignore the protocol by checking explicitly for
http://
orhttps://
, instead of doing it by starting at position 8 in the string.Also since the Location values in the form
/f/rEeTUL8MXR17A
do not contain theroot
domain, it would have to be retrieved from somewhere else (just trying the current domain in the request URL should work), as currently it's just the first element of the split.The text was updated successfully, but these errors were encountered: