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

Fix RSS enclosure parsing #218

Merged
merged 4 commits into from
Feb 7, 2024
Merged

Fix RSS enclosure parsing #218

merged 4 commits into from
Feb 7, 2024

Conversation

sgodart
Copy link
Contributor

@sgodart sgodart commented Dec 22, 2023

Fix #217
Ignore all child nodes of <enclosure>, not only TextNodes.

sgodart and others added 2 commits December 22, 2023 12:19
@mmcdole
Copy link
Owner

mmcdole commented Dec 22, 2023

@sgodart can you include a test that demonstrates the issue at hand you are encountering?

I saw your issue you created. A test where it fails and is broken, and this PR fixes, would be much appreciated.

@sgodart
Copy link
Contributor Author

sgodart commented Jan 3, 2024

Hey @mmcdole, thanks for your reply.
Here is a code snippet to reproduce, you should see "Error parsing feed..." in stdout :

package main

import (
	"fmt"

	"github.com/mmcdole/gofeed"
)

func main() {
	parser := gofeed.NewParser()
	_, err := parser.ParseURL("https://sportauto.autojournal.fr/feed")

	if err != nil {
		fmt.Printf("Error parsing feed: %s\n", err.Error())
	} else {
		fmt.Printf("No error\n")
	}
}

The same snippet using this PR should output "No error", adding this statement in your go.mod:

replace github.com/mmcdole/gofeed => github.com/ividence/gofeed v1.2.2

@sgodart
Copy link
Contributor Author

sgodart commented Feb 1, 2024

hey @mmcdole, any update on this?

@mmcdole
Copy link
Owner

mmcdole commented Feb 7, 2024

@sgodart greetings!

I appreciate the further details, but I was really asking for a new unit test, if you could, along with the patch.

They are specified with minimal feed representations and the expected output. So, for this, it would be a feed with an enclosure with a media tag (or similar), and then the specified parsed output.

https://github.com/mmcdole/gofeed/tree/master/testdata/parser/rss

That helps ensure any refactoring or future changes catch this issue as well and that we don't regress.

@sgodart
Copy link
Contributor Author

sgodart commented Feb 7, 2024

Hey @mmcdole, thanks again for your help.
I've added the test files.
Have a nice day.

@mmcdole mmcdole merged commit 38e4aa4 into mmcdole:master Feb 7, 2024
1 check failed
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.

Ignore RSS "enclosure" child tags
2 participants