-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
No pubDate for RSS messes up etags for the feed #2777
Comments
@andris9 Not really sure about that, but looking at our RSS feed from Ghostalk, Ghost does provide the It is even taken from the date of the most recent post. What version are you running? |
@halfdan Does appear to occur on self hosted running tag 0.4.2, as I can see my site doing the same. I'm at work so I can't dig much further at the moment. I'll dig into a bit further in a couple of hours though. |
I looked further anyway lol, shhh https://github.com/dylang/node-rss/blob/master/lib/rss.js#L75 |
Right, so most of that is going to be fixed once I ship my own RSS library
|
@halfdan Rushed release of your library? j/k Any idea what they are using on the hosted site? (Or is it your library?) |
Hosted is cached by cloudflare. My library isn't close to done yet - will
|
Should be addressed in #2263 |
Going to close this against #2263 as there seems little point in having both issues open. |
#3993 should get this resolved |
RSS module takes an optional property
pubDate
which is used for<lastBuildDate>
header in the RSS XML. Ghost does not provide this value and current time is used by default. This in turn messes up HTTP response ETag headers – as the RSS feed seems to be "different" a new ETag is generated for every request. ThusIf-None-Match
requests do not work. Making ETags work properly would make it possible to return an empty 304 response to such query instead of the full RSS feed.Proposed solution: use the date of the newest message as
pubDate
for the feed.The text was updated successfully, but these errors were encountered: