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

Link feed field recommended, but not required for Atom and JSON feeds #52

Closed
pruser opened this issue Jun 24, 2018 · 6 comments
Closed
Labels

Comments

@pruser
Copy link

pruser commented Jun 24, 2018

I noticed "runtime error: invalid memory address or nil pointer dereference" while using feeds library, so i started checking the issue.
It turned out that the library assumes that Feed.Link field is set although it has pointer type.

After checking the specs:
The Link field is required only for RSS feeds.
Atom feeds does not require feed to contain element, but it's recommended though.
JSON feed spec state that home_page_url and feed_url is optional, but strongly recommended.

I can fix this issue for Atom and JSON feeds, but I'd like to know your opinion first.

Would you like to keep it as it is, or to create non required fields only if Feed.Link is set?

specs:
https://validator.w3.org/feed/docs/atom.html
https://jsonfeed.org/version/1

@kisielk
Copy link
Contributor

kisielk commented Nov 13, 2018

If you could provide a PR or reproducible example, that would be helpful. Thanks!

@stale
Copy link

stale bot commented Jan 12, 2019

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

@pruser
Copy link
Author

pruser commented Jan 12, 2019

I just realised that gorilla feeds use Feed.Link field for generating Atom channel Id according to the definition of Id field.

@stale stale bot removed the stale label Jan 12, 2019
@stale
Copy link

stale bot commented Mar 13, 2019

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

@stale stale bot added the stale label Mar 13, 2019
@stale stale bot closed this as completed Mar 20, 2019
@preslavrachev
Copy link

What is the state of this issue? I just recently came to the same problem.

@pruser
Copy link
Author

pruser commented May 12, 2019

Even though the 'link' element is not required by Atom feed spec the 'id' is. The purpose of 'id' field is to contain unique and permanent URI, so I think it can be used to create target feed object from gorilla 'Feed' object.

I modified conversion logic to return error instead of crashing on null dereference.
#61 .
The PR was not accepted by maintainers and was closed automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants