From 2d79d4cbd5b665a062d4b2dad2362413aaa17f2a Mon Sep 17 00:00:00 2001
From: jbenezech <jeromekampot@gmail.com>
Date: Wed, 12 Oct 2022 20:11:19 +0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20sitemaps=20with=20no=20c?=
 =?UTF-8?q?ontent=20(#15571)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes: https://github.com/TryGhost/Ghost/issues/14981

- Taxonomy-specific sitemaps were invalid xml when there was no data
- These invalid empty sitemaps were referenced in the index sitemap causing SEO tools to report errors
---
 .../core/frontend/services/sitemap/handler.js    |  2 +-
 .../frontend/services/sitemap/index-generator.js |  4 +---
 .../core/test/e2e-frontend/custom_routes.test.js |  6 ++++++
 .../frontend/services/sitemap/generator.test.js  | 16 ++++++++++++++++
 .../test/utils/fixtures/settings/newroutes.yaml  |  1 -
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/ghost/core/core/frontend/services/sitemap/handler.js b/ghost/core/core/frontend/services/sitemap/handler.js
index 817eee1e461..4898b6b39cf 100644
--- a/ghost/core/core/frontend/services/sitemap/handler.js
+++ b/ghost/core/core/frontend/services/sitemap/handler.js
@@ -30,7 +30,7 @@ module.exports = function handler(siteApp) {
         const content = manager.getSiteMapXml(type, page);
         // Prevent x-1.xml as it is a duplicate of x.xml and empty sitemaps
         // (except for the first page so that at least one sitemap exists per type)
-        if (pageParam === '1' || (!content && page !== 1)) {
+        if (pageParam === '1' || content === null) {
             return res.sendStatus(404);
         }
 
diff --git a/ghost/core/core/frontend/services/sitemap/index-generator.js b/ghost/core/core/frontend/services/sitemap/index-generator.js
index d0c1d107d40..fb6b46909ab 100644
--- a/ghost/core/core/frontend/services/sitemap/index-generator.js
+++ b/ghost/core/core/frontend/services/sitemap/index-generator.js
@@ -31,10 +31,8 @@ class SiteMapIndexGenerator {
 
     generateSiteMapUrlElements() {
         return _.map(this.types, (resourceType) => {
-            // `|| 1` = even if there are no items we still have an empty sitemap file
-            const noOfPages = Math.ceil(Object.keys(resourceType.nodeLookup).length / this.maxPerPage) || 1;
+            const noOfPages = Math.ceil(Object.keys(resourceType.nodeLookup).length / this.maxPerPage);
             const pages = [];
-
             for (let i = 0; i < noOfPages; i++) {
                 const page = i === 0 ? '' : `-${i + 1}`;
                 const url = urlUtils.urlFor({relativeUrl: '/sitemap-' + resourceType.name + page + '.xml'}, true);
diff --git a/ghost/core/test/e2e-frontend/custom_routes.test.js b/ghost/core/test/e2e-frontend/custom_routes.test.js
index 426e443a8db..7c4977662ce 100644
--- a/ghost/core/test/e2e-frontend/custom_routes.test.js
+++ b/ghost/core/test/e2e-frontend/custom_routes.test.js
@@ -73,4 +73,10 @@ describe('Custom Frontend routing', function () {
             .expect(200)
             .expect(assertCorrectFrontendHeaders);
     });
+
+    it('should not serve empty sitemaps', async function () {
+        await request.get('/sitemap-authors.xml')
+            .expect(404)
+            .expect(assertCorrectFrontendHeaders);
+    });
 });
diff --git a/ghost/core/test/unit/frontend/services/sitemap/generator.test.js b/ghost/core/test/unit/frontend/services/sitemap/generator.test.js
index a50dd5655b1..7491e197889 100644
--- a/ghost/core/test/unit/frontend/services/sitemap/generator.test.js
+++ b/ghost/core/test/unit/frontend/services/sitemap/generator.test.js
@@ -93,6 +93,11 @@ describe('Generators', function () {
 
         describe('fn: getXml', function () {
             it('default', function () {
+                generator.types.posts.addUrl('http://my-ghost-blog.com/episode-1/', {id: 'identifier1', staticRoute: true});
+                generator.types.pages.addUrl('http://my-ghost-blog.com/home/', {id: 'identifier1', staticRoute: true});
+                generator.types.tags.addUrl('http://my-ghost-blog.com/home/', {id: 'identifier1', staticRoute: true});
+                generator.types.authors.addUrl('http://my-ghost-blog.com/home/', {id: 'identifier1', staticRoute: true});
+    
                 const xml = generator.getXml();
 
                 xml.should.match(/sitemap-tags.xml/);
@@ -101,6 +106,17 @@ describe('Generators', function () {
                 xml.should.match(/sitemap-authors.xml/);
             });
 
+            it('does not create entries for pages with no content', function () {
+                generator.types.tags.addUrl('http://my-ghost-blog.com/episode-1/', {id: 'identifier1', staticRoute: true});
+    
+                const xml = generator.getXml();
+
+                xml.should.match(/sitemap-tags.xml/);
+                xml.should.not.match(/sitemap-posts.xml/);
+                xml.should.not.match(/sitemap-pages.xml/);
+                xml.should.not.match(/sitemap-authors.xml/);
+            });
+
             it('creates multiple pages when there are too many posts', function () {
                 for (let i = 0; i < 10; i++) {
                     generator.types.posts.addUrl(`http://my-ghost-blog.com/episode-${i}/`, testUtils.DataGenerator.forKnex.createPost({
diff --git a/ghost/core/test/utils/fixtures/settings/newroutes.yaml b/ghost/core/test/utils/fixtures/settings/newroutes.yaml
index 258184aeb0e..ae4e2645c33 100644
--- a/ghost/core/test/utils/fixtures/settings/newroutes.yaml
+++ b/ghost/core/test/utils/fixtures/settings/newroutes.yaml
@@ -8,4 +8,3 @@ collections:
 
 taxonomies:
   tag: /category/{slug}/
-  author: /author/{slug}/