-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix the Syndication Feed RFC822 DateTimeParser #99194
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
Conversation
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.
...raries/System.ServiceModel.Syndication/src/System/ServiceModel/Syndication/DateTimeHelper.cs
Outdated
Show resolved
Hide resolved
...raries/System.ServiceModel.Syndication/src/System/ServiceModel/Syndication/DateTimeHelper.cs
Show resolved
Hide resolved
|
Any chance on getting this merged for .NET 9? |
|
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. |
There was a problem hiding this 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 updates the date-time format patterns in the RFC822 parser to correctly handle single-digit day values and two-digit year values as per the RFC822 specification.
- Updated date format strings to use "d" instead of "dd" for days.
- Adjusted both the time formats with seconds and without seconds to support the new behavior.
Comments suppressed due to low confidence (2)
src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/Syndication/DateTimeHelper.cs:95
- Ensure that unit tests cover the new date-time formats, particularly for cases with single-digit day values and two-digit years.
"ddd, d MMMM yyyy HH:mm:ss zzz",
src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/Syndication/DateTimeHelper.cs:105
- [nitpick] Consider reviewing the ordering of the format patterns to ensure that the most frequently encountered date formats are prioritized for optimal performance.
// The original RFC822 spec listed 2 digit years. RFC1123 updated the format to include 4 digit years and states that you should use 4 digits.
StephenMolloy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
@dotnet-policy-service rerun |
|
/azp run runtime |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Thanks @dougwaldron! This is a solid improvement that brings our RSS date handling closer to the intent of RFC 822 while keeping the existing layered fallback behavior. For anyone following along: the current RSS date parsing path (see the internal helper in System.ServiceModel.Syndication) tries, in order:
That balance lets us stay compliant yet resilient. This PR tightens conformance in the RFC 822 layer without removing the practical fallbacks. Reminder: If you encounter feeds whose date strings are even more “creative” (e.g., ordinal day tokens, missing commas, non-English month names, 2‑digit years, or embedded comments), you do NOT have to push further loosening into the core library. You can plug in your own parsing logic via the public delegate hook: using System;
using System.ServiceModel.Syndication;
// Do this once early in app startup (before loading feeds).
// Capture the existing parser so you can chain/fallback.
var originalRssParser = Rss20FeedFormatter.DateTimeParser;
// Install a custom parser
Rss20FeedFormatter.DateTimeParser = (xmlDateTimeData, out DateTimeOffset dto) =>
{
// xmlDateTimeData.DateTimeString is the raw value.
// Return true if you successfully parsed; set dto accordingly.
if (MyVeryForgivingParser(xmlDateTimeData.DateTimeString, out dto))
{
return true;
}
// Fall back to the framework’s (now improved) default behavior
return originalRssParser(xmlDateTimeData, out dto);
};
// Example custom routine (simplified)
static bool MyVeryForgivingParser(string s, out DateTimeOffset dto)
{
// Your heuristics here (strip ordinal suffixes, normalize TZ tokens, etc.)
// Return true on success.
dto = default;
return false;
}(Atom feeds have an analogous hook via A few tips when customizing:
Given the clear standards improvement and low risk, I’m proceeding with the merge. Thanks again for tightening this up and helping the ecosystem stay closer to the spec while still allowing extensibility. |
|
/backport to release/10.0 |
|
Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/17871053134 |
* Fix the Syndication RFC822 DateTimeParser 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. * Remove trailing whitespace in parseFormat array
RFC822 allows single-digit date values and two-digit year values in the publication date (
pubDate). The existingRfc822DateTimeParsermethod inSystem.ServiceModel.Syndication.DateTimeHelperonly accepts two-digit dates and (for some formats) four-digit years.This PR updates the list of date-time formats.
Fixes #99193