PTP: fix poster rehosting - #1375
Conversation
|
Thanks for taking the time to contribute to this project. Upload Assistant is currently in a complete rewrite, and no new development is being conducted on this python source at this time. If you have come this far, please feel free to leave open, any pull requests regarding new sites being added to the source, as these can serve as the baseline for later conversion. If your pull request relates to a critical bug, this will be addressed in this code base, and a new release published as needed. If your pull request only addresses a quite minor bug, it is not likely to be addressed in this code base. Details for the new code base will follow at a later date. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPTP poster/cover rehosting was refactored to replace deprecated ptpimg-specific logic with configurable host selection. New helper methods determine the target imghost from metadata or config, detect existing host matches via alias rules, derive file extensions, and orchestrate rehosting. The cover acquisition flow now validates user-provided poster URLs and rehosts to the selected host. ChangesPoster Rehosting Infrastructure and Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/trackers/PTP.py`:
- Around line 480-482: _selected_poster_host currently returns any configured
imghost/default (e.g., img_host_1) but must be constrained to only PTP-approved
hosts; change _selected_poster_host to validate the chosen host against
self.approved_image_hosts and only return it when it is present in that
set/list, otherwise return an empty string (or None) so callers won’t rehost to
an unapproved host; apply the same membership check to the analogous poster-host
selection logic around the other block referenced (lines ~516-535) so both
places only accept hosts in self.approved_image_hosts.
- Around line 1578-1587: The code treats an empty string as a valid poster
string causing rehost_poster_to_selected_host to return '' and the subsequent
while cover is None loop never runs; update the branch around cover and the call
to rehost_poster_to_selected_host so blank/whitespace-only strings are treated
as missing: change the condition to check for a non-empty string (e.g., if
isinstance(cover, str) and cover.strip():) before calling
rehost_poster_to_selected_host(meta, cover), otherwise set cover = None so the
while cover is None loop will prompt for cover_input and
rehost_poster_to_selected_host is only called with a non-empty URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| def _selected_poster_host(self, meta: dict[str, Any]) -> str: | ||
| default_config = cast(dict[str, Any], self.config.get('DEFAULT', {})) | ||
| return str(meta.get('imghost') or default_config.get('img_host_1') or '').strip() |
There was a problem hiding this comment.
Constrain poster rehosting to PTP-approved hosts.
_selected_poster_host() accepts any global/default imghost, but this tracker only approves self.approved_image_hosts. With a config like img_host_1=imgbb, this path will upload the poster to an unsupported host and then submit that URL in image, which regresses uploads for users whose global host is not PTP-safe.
Suggested guard
def _selected_poster_host(self, meta: dict[str, Any]) -> str:
default_config = cast(dict[str, Any], self.config.get('DEFAULT', {}))
- return str(meta.get('imghost') or default_config.get('img_host_1') or '').strip()
+ for candidate in (
+ str(meta.get('imghost') or '').strip().lower(),
+ str(default_config.get('img_host_1') or '').strip().lower(),
+ ):
+ if candidate in self.approved_image_hosts:
+ return candidate
+ return ''Also applies to: 516-535
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/trackers/PTP.py` around lines 480 - 482, _selected_poster_host currently
returns any configured imghost/default (e.g., img_host_1) but must be
constrained to only PTP-approved hosts; change _selected_poster_host to validate
the chosen host against self.approved_image_hosts and only return it when it is
present in that set/list, otherwise return an empty string (or None) so callers
won’t rehost to an unapproved host; apply the same membership check to the
analogous poster-host selection logic around the other block referenced (lines
~516-535) so both places only accept hosts in self.approved_image_hosts.
fixes #1369
Summary by CodeRabbit