-
Notifications
You must be signed in to change notification settings - Fork 3.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
Feat: Mediafire Hoster #1420
base: main
Are you sure you want to change the base?
Feat: Mediafire Hoster #1420
Conversation
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Added Mediafire as a new download hoster with URL parsing and download link extraction capabilities, integrated into the existing download manager infrastructure.
- Added
MediafireApi
class in/src/main/services/hosters/mediafire.ts
to handle both direct redirects and HTML page parsing for download links - Added Mediafire to
Downloader
enum in/src/shared/constants.ts
and integrated with download manager's payload handling - Potential issue: URL pattern mismatch between
mediafire.com
in realDebridHosts andwww.mediafire.com
check could cause incorrect routing - Missing error handling in
MediafireApi.getDownloadUrl()
for cases where download button or location header is not found - User-Agent header hardcoded to Chrome 125 in MediafireApi may need maintenance as browser versions change
💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
6 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
case Downloader.Mediafire: { | ||
const downloadUrl = await MediafireApi.getDownloadUrl(game.uri!); | ||
|
||
return { | ||
action: "start", | ||
game_id: game.id, | ||
url: downloadUrl, | ||
save_path: game.downloadPath!, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: missing error handling if MediafireApi.getDownloadUrl fails
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36", | ||
}, | ||
maxRedirects: 0, | ||
validateStatus: (status: number) => status === 302 || status < 400, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: This validation allows any status code < 400, which could allow unexpected 3xx redirects. Should explicitly list allowed status codes.
); | ||
|
||
if (response.status === 302) { | ||
return response.headers["location"] || ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Empty string fallback could silently fail. Should throw an error if location header is missing on 302 response.
"a#downloadButton" | ||
) as HTMLAnchorElement; | ||
|
||
return downloadButton?.href || ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Empty string fallback could mask errors. Should throw if download button is not found in HTML.
@@ -88,6 +88,7 @@ export const getDownloadersForUri = (uri: string) => { | |||
if (uri.startsWith("https://pixeldrain.com")) return [Downloader.PixelDrain]; | |||
if (uri.startsWith("https://qiwi.gg")) return [Downloader.Qiwi]; | |||
if (uri.startsWith("https://datanodes.to")) return [Downloader.Datanodes]; | |||
if (uri.startsWith("https://www.mediafire.com")) return [Downloader.Mediafire]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: check should be uri.includes('mediafire.com') to catch all Mediafire URLs regardless of subdomain
When submitting this pull request, I confirm the following (please check the boxes):
Fill in the PR content: