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 the Syndication Feed RFC822 DateTimeParser #99194

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dougwaldron
Copy link

RFC822 allows single-digit date values and two-digit year values in the publication date (pubDate). The existing Rfc822DateTimeParser method in System.ServiceModel.Syndication.DateTimeHelper only accepts two-digit dates and (for some formats) four-digit years.

This PR updates the list of date-time formats.

Fixes #99193

RFC822 allows single-digit date values and two-digit year values in the publication date (pubDate). The existing date-time parser only accepted two-digit dates and (for some formats) four-digit years.
"d MMMM yyyy HH:mm:ss zzz",
"ddd, d MMM yyyy HH:mm:ss zzz",
"d MMM yyyy HH:mm:ss zzz",

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Comment on lines -100 to -103
"ddd, dd MMMM yyyy HH:mm zzz",
"dd MMMM yyyy HH:mm zzz",
"ddd, dd MMM yyyy HH:mm zzz",
"dd MMM yyyy HH:mm zzz",
Copy link
Member

@filipnavara filipnavara Mar 2, 2024

Choose a reason for hiding this comment

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

This is removing the overloads without the :ss part, why? (seconds are optional in the time-of-day ABNF in RFC 5322)

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure what you mean; I didn't remove any overloads or change the :ss part. All the previously existing formats are still there, just with the correct number of digits for date and year.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, sorry, it was hidden under the messages about whitespace error.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, that makes sense, there were a LOT of error messages! 😅

@dougwaldron
Copy link
Author

Any chance on getting this merged for .NET 9?

@dougwaldron
Copy link
Author

Any chance of getting this merged for 9.0.2?

This is very simple fix for a bug caused by a small typo. There is almost zero risk to merging this, and it would fix a bug that has existed in my application for a year now.

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

Successfully merging this pull request may close these issues.

The Syndication DateTimeHelper class throws an exception on some valid RFC822 dates
2 participants