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

Undocumented / Unwanted? API change in Version 0.9 #95

Closed
kittel opened this issue Feb 10, 2020 · 2 comments
Closed

Undocumented / Unwanted? API change in Version 0.9 #95

kittel opened this issue Feb 10, 2020 · 2 comments

Comments

@kittel
Copy link

kittel commented Feb 10, 2020

The latest published version (0.9) contains an unwanted? API change. The getter function summary() in FeedEntry is now returning a dict instead of a string.

def summary(self, summary=None, type=None):

See the following example reading the Summary of an entry:

Expected behavior: Return Summary as String

Actual behavior: Summary is returned as dict -> {'summary': "content"}

Steps to reproduce:

  • Create test.py in current folder:
from feedgen.feed import FeedGenerator
from feedgen.version import version as feedgenversion
print('Feedgen Version: {}'.format(feedgenversion))
feed = FeedGenerator()
fe = feed.add_entry()
fe.summary('description')
print('Return type:     {}'.format(type(feed.entry()[0].summary())))
print("Summary:         {}".format(feed.entry()[0].summary()))
  • Execute the following command
pip3 install --user --upgrade feedgen==0.8 > /dev/null; python3 test.py; echo "----------"; pip3 install --user --upgrade feedgen==0.9 > /dev/null; python3 test.py
  • Receive the following output:
Feedgen Version: (0, 8, 0)
Return type:     <class 'str'>
Summary:         description
----------
Feedgen Version: (0, 9, 0)
Return type:     <class 'dict'>
Summary:         {'summary': 'description'}
@kittel
Copy link
Author

kittel commented Mar 4, 2020

Hello again,

as there is a documented CVE in feedgen version 0.8 [0] i wonder when there will be a reaction to this. Would it help to propose a fix and submit it as a merge request?

regards

[0] https://nvd.nist.gov/vuln/detail/CVE-2020-5227

@lkiesow
Copy link
Owner

lkiesow commented Dec 24, 2023

You are absolutely right that this is not great. Unfortunately, that's a design decision from right at the beginning of the project which isn't easy to fix. The methods always return the internal storage structure. This may be a string (if only a string is being stored), but will often be a dictionary.

For example, this:

fe.summary('description')
fe.source('http://example.com', 'Example')
fe.content('Some text')
print("Summary:  {}".format(feed.entry()[0].summary()))
print("Source:   {}".format(feed.entry()[0].source()))
print("Content:  {}".format(feed.entry()[0].content()))

will result in:

Summary:  {'summary': 'description'}
Source:   {'url': 'http://example.com', 'title': 'Example'}
Content:  {'content': 'Some text'}

Summary allows you to store a type as well as the summary itself. Returning only the summary would mean that there is no way to see what type is stored for the summary, which then would be inconsistent with the other methods.

That being said, I'll try to add changes like these to release notes.

@lkiesow lkiesow closed this as completed Dec 24, 2023
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

No branches or pull requests

2 participants