Skip to content

Conversation

Grotax
Copy link

@Grotax Grotax commented Jul 23, 2025

Import of alexdebril#433
Also cherry-picked the adjustments from the PR for 5.3.x

@Grotax Grotax requested review from SMillerDev and Copilot July 23, 2025 12:10
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the analysis of relative links in Atom feeds by improving URL resolution logic. The changes ensure that relative URLs in feed items are properly converted to absolute URLs using the base feed URL.

  • Adds comprehensive relative URL resolution logic for Atom links
  • Introduces a separate linkForAnalysis property to track the original feed URL for relative URL resolution
  • Updates content processing to handle various types of relative URLs (fragment, query, path-based)

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/FeedIo/Rule/Atom/Link.php Implements relative URL resolution by checking if href has a host and prepending base URL
src/FeedIo/Feed/Node.php Adds linkForAnalysis property and enhanced setHostInContent method for URL processing
src/FeedIo/Parser/XmlParser.php Updates to use setLinkForAnalysis instead of setLink for item parsing
tests/FeedIo/Rule/Atom/LinkTest.php Adds comprehensive test coverage for various relative URL scenarios
CHANGELOG.md Documents the fix for relative links analysis
Comments suppressed due to low confidence (1)

src/FeedIo/Feed/Node.php:187

  • The regex pattern has unbalanced parentheses - there are 6 opening parentheses but only 5 closing parentheses. This will cause a preg_replace error. The pattern should be properly balanced.
        $pattern = '(<\s*[^>]*)(href=|src=)(.?)(\w+\b)(?![:])(?!(.(?!<code))*<\/code>)';

IgorA100 and others added 3 commits August 2, 2025 20:01
1 Cannot use the "Link" object to analyze relative links
Because the "Link" for the Atom feed will be corrupted.
Use the additionally created "LinkForAnalysis" object

2.
- Fix: Error preg_replace(), add all other possible replacements for relative links.
- Replaced links like href="#aaa/bbb.xxx"
- Replaced links like href="aaa/bbb.xxx"
Similar to alexdebril#427

Use the new method "setLinkForAnalysis" instead of "setLink" (XmlParser.php)

Added analysis of relative links for the Atom feed (Link.php)
Co-authored-by: Sean Molenaar <[email protected]>
@Grotax
Copy link
Author

Grotax commented Aug 5, 2025

@SMillerDev tests are still failing after your changes

@Grotax Grotax requested a review from SMillerDev August 8, 2025 08:05
@Grotax Grotax merged commit e465b46 into main Aug 13, 2025
4 checks passed
@Grotax Grotax deleted the import-pr-433 branch August 13, 2025 09:32
@Grotax
Copy link
Author

Grotax commented Aug 13, 2025

I removed the problematic commits, maybe code quality can be improved another time

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.

3 participants