diff --git a/README.md b/README.md index 231e6b0..d531d4f 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,10 @@ Then tell crawlers where the index is, in `public/robots.txt`: Sitemap: https://www.example.com/sitemap.xml ``` +When the sitemaps are served from elsewhere (`config.sitemaps_url`, see [Storage](#storage)), that line names +the index there: a sitemap on another host, or in a folder of the site, may list the pages of the site only +when its `robots.txt` points to it. + ## Sources A source is a class in `app/sitemaps`. Its blocks run in an object that has your route helpers, already set to @@ -94,7 +98,10 @@ end ``` A page in one language gets no `hreflang` link. A page that does not exist in the `x_default` language gets -no `x-default` link. +no `x-default` link. A Rails locale such as `pt_BR` is written `hreflang="pt-BR"`, and two languages that share +a URL list it once. + +A URL your block writes by hand is percent-encoded: spaces, accents and control characters cannot break a file. ### When the date depends on more than one column @@ -205,9 +212,15 @@ cannot be reached is a warning, never a failed build. bin/rails mapitaine:check ``` -It renders every sitemap without publishing or submitting anything, and fails on a URL outside `config.host`, -a URL listed twice in a source, or a `lastmod` in the future. A build reports the same, for the files it -rewrites, in `warnings`. +It renders every sitemap without publishing or submitting anything, and fails on: + +- a URL outside `config.host`, listed twice in a source, or longer than 2,048 characters, and a page whose + languages all share one URL; +- an `hreflang` that is not a language code (`en`, `pt-BR`, `zh-Hant`, `es-419`); +- a `lastmod` in the future, or one that is not a W3C date, which is then left out; +- an index with more than the 50,000 files it may hold. + +A build reports the same, for the files it rewrites, in `warnings`. ## Instrumentation diff --git a/docs/guide.html b/docs/guide.html index fe0f027..b471b88 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -61,6 +61,7 @@

Installation

end

Then tell crawlers where the index is, in public/robots.txt:

Sitemap: https://www.example.com/sitemap.xml
+

When the sitemaps are served from elsewhere (config.sitemaps_url, see Storage), that line names the index there: a sitemap on another host, or in a folder of the site, may list the pages of the site only when its robots.txt points to it.

@@ -98,7 +99,8 @@

Languages

url { |product, locale| product_url(product, locale:) } locales { |product| product.translated_locales } end -

Each <url> then lists every language of its page, itself included, as hreflang links. A page in one language gets no link. A page that does not exist in the x_default language gets no x-default link.

+

Each <url> then lists every language of its page, itself included, as hreflang links. A page in one language gets no link. A page that does not exist in the x_default language gets no x-default link. A Rails locale such as pt_BR is written hreflang="pt-BR", and two languages that share a URL list it once.

+

A URL your block writes by hand is percent-encoded: spaces, accents and control characters cannot break a file.

@@ -163,7 +165,7 @@

Through Active Storage

config.sitemaps_url = "https://cdn.example.com/sitemaps" # where those files are served from end

No blob is created: the files keep their names as keys, and nothing is written to the database. Any service works that serves a key at a stable URL: S3, Google Cloud Storage, Azure, a mirror of them. The Disk service does not, it scatters keys in subfolders: keep the default for files on disk.

-

Sitemaps served from another host than the pages they list are accepted when robots.txt names the index.

+

Sitemaps served from another host than the pages they list are accepted when the site's robots.txt names the index there.

Your own

A storage is any object with read(name), write(name, path) and delete(name).

@@ -183,7 +185,14 @@

IndexNow

Checking

bin/rails mapitaine:check
-

It renders every sitemap without publishing or submitting anything, and fails on a URL outside config.host, a URL listed twice in a source, or a lastmod in the future. A build reports the same in warnings, for the files it rewrites.

+

It renders every sitemap without publishing or submitting anything, and fails on:

+ +

A build reports the same in warnings, for the files it rewrites.

Mapitaine never requests a page of the site. Whether a URL answers 200 or carries a noindex is what Search Console reports, from the visits crawlers make anyway.

diff --git a/lib/mapitaine.rb b/lib/mapitaine.rb index 696b9a5..fec6bda 100644 --- a/lib/mapitaine.rb +++ b/lib/mapitaine.rb @@ -4,10 +4,12 @@ require "active_support/core_ext/object/blank" require "active_support/core_ext/string/inflections" require "active_support/notifications" +require "date" require "digest" require "fileutils" require "json" require "net/http" +require "time" require "tmpdir" require "uri" require "zlib" diff --git a/lib/mapitaine/builder.rb b/lib/mapitaine/builder.rb index 4b66dea..8437549 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -8,6 +8,12 @@ def changed? end end + # https://www.sitemaps.org/protocol.html + MAX_URL_CHARACTERS = 2_048 + MAX_FILES_PER_INDEX = 50_000 + # A language, then maybe a script and a region: en, pt-BR, zh-Hant-TW, es-419. + HREFLANG = /\A[a-z]{2,3}(-[a-z]{4})?(-([a-z]{2}|\d{3}))?\z/i + Warning = Data.define(:source, :url, :message) do def to_s "#{source}: #{url} #{message}" @@ -37,6 +43,9 @@ def build_in(directory, full:) @kept = [] @warnings = [] @changed_urls = [] + @checked_hreflangs = Set.new + @reported = Set.new + @x_default = Xml.hreflang(@configuration.x_default) @configuration.sources.each { |source_class| build_source(source_class.new(@configuration)) } publish_index @@ -71,7 +80,7 @@ def render(source, shard, fingerprint, stored, seen_urls) ActiveSupport::Notifications.instrument("shard.mapitaine", source: source.slug, shard:) do source.each_page(shard) do |page| inspect_page(source, page, seen_urls) - writer.add(Xml.url_entries(page, x_default: @configuration.x_default), Xml.timestamp(page.lastmod)) + writer.add(Xml.url_entries(page, x_default: @x_default), page.lastmod) changed_urls.concat(page.urls) if changed_since_last_build?(page) end writer.close @@ -96,10 +105,12 @@ def publish(source, shard, fingerprint, files) end def publish_index + name = "#{@configuration.filename}.xml" entries = @manifest.files.map do |file| Xml.sitemap_entry("#{@configuration.sitemaps_url}/#{file["name"]}", file["lastmod"]) end - upload("#{@configuration.filename}.xml", "#{Xml::INDEX_OPEN}#{entries.join}#{Xml::INDEX_CLOSE}") + inspect_index("#{@configuration.sitemaps_url}/#{name}", entries.size) + upload(name, "#{Xml::INDEX_OPEN}#{entries.join}#{Xml::INDEX_CLOSE}") end def upload(name, content) @@ -116,19 +127,43 @@ def inspect_page(source, page, seen_urls) page.urls.each do |url| warn(source, url, "is outside #{@configuration.host!}") unless url.start_with?("#{@configuration.host!}/") || url == @configuration.host! warn(source, url, "is listed twice") unless seen_urls.add?(url) + warn(source, url, "is longer than #{MAX_URL_CHARACTERS} characters") if url.length > MAX_URL_CHARACTERS + end + page.url_by_locale.each do |hreflang, url| + warn(source, url, "has hreflang #{hreflang.inspect}, not a language code") if @checked_hreflangs.add?(hreflang) && !hreflang.match?(HREFLANG) + end + if page.url_by_locale.size > 1 && page.urls.one? + warn_once(source, :one_url, page.urls.first, "is the URL of every language of its page: does the url block use the locale?") + end + inspect_lastmod(source, page) + end + + def inspect_lastmod(source, page) + if page.rejected_lastmod + warn_once(source, :rejected_lastmod, page.urls.first, "has a lastmod that is not a W3C date (#{page.rejected_lastmod.inspect}), left out") + elsif page.modified_at && page.modified_at > @manifest.built_at + warn(source, page.urls.first, "has a lastmod in the future (#{page.lastmod})") end - modified_at = page.modified_at - warn(source, page.urls.first, "has a lastmod in the future (#{Xml.timestamp(page.lastmod)})") if modified_at && modified_at > @manifest.built_at + end + + def inspect_index(url, file_count) + return if file_count <= MAX_FILES_PER_INDEX + + warn("index", url, "lists #{file_count} files, more than #{MAX_FILES_PER_INDEX}: raise shard_size") + end + + # For a mistake every page of a source would repeat. + def warn_once(source, kind, url, message) + warn(source, url, message) if @reported.add?([source.slug, kind]) end def warn(source, url, message) - @warnings << Warning.new(source: source.slug, url:, message:) + @warnings << Warning.new(source: source.is_a?(Source) ? source.slug : source, url:, message:) end # The first build has no "before": submitting a whole site at once is not what IndexNow is for. def changed_since_last_build?(page) - modified_at = page.modified_at - @previous.built_at && modified_at && modified_at >= @previous.built_at + @previous.built_at && page.modified_at && page.modified_at >= @previous.built_at end def submit_changed_urls diff --git a/lib/mapitaine/page.rb b/lib/mapitaine/page.rb index 3e9962c..ab62e43 100644 --- a/lib/mapitaine/page.rb +++ b/lib/mapitaine/page.rb @@ -1,14 +1,11 @@ # frozen_string_literal: true module Mapitaine - # One page of the site: the same content at one URL per language. - Page = Data.define(:url_by_locale, :lastmod, :images) do + # One page of the site: the same content at one URL per language, keyed by hreflang. lastmod is a W3C date; + # rejected_lastmod what a source gave that was not one. + Page = Data.define(:url_by_locale, :lastmod, :modified_at, :images, :rejected_lastmod) do def urls - url_by_locale.values - end - - def modified_at - lastmod.to_time if lastmod.respond_to?(:to_time) + url_by_locale.values.uniq end end end diff --git a/lib/mapitaine/source.rb b/lib/mapitaine/source.rb index 75dd4b2..87c3365 100644 --- a/lib/mapitaine/source.rb +++ b/lib/mapitaine/source.rb @@ -16,6 +16,11 @@ class Source SLUG_FORMAT = /\A[a-z0-9][a-z0-9_-]*\z/ # A source that is not an Active Record relation is one shard, rendered at each build. WHOLE = "all" + # What RFC 3986 lets a URL hold as is. Anything else is percent-encoded, byte by byte. + URL_CHARACTERS = "A-Za-z0-9\\-._~:/?#\\[\\]@!$&'()*+,;=%" + UNSAFE = "[^#{URL_CHARACTERS}]|%(?![0-9A-Fa-f]{2})" + UNSAFE_CHARACTER = Regexp.new(UNSAFE) + UNSAFE_BYTE = Regexp.new(UNSAFE, Regexp::NOENCODING) class << self def inherited(subclass) @@ -144,13 +149,29 @@ def page_for(record) locales = definitions[:locales] ? Array(instance_exec(record, &definitions[:locales])) : configuration.locales url_by_locale = locales.filter_map do |locale| location = instance_exec(record, locale, &definitions.fetch(:url)) - [locale, absolute(location)] if location + [Xml.hreflang(locale), absolute(location)] if location end - Page.new(url_by_locale: url_by_locale.to_h, lastmod: lastmod_of(record), images: images_of(record)) + given_lastmod = lastmod_of(record) + lastmod = Xml.timestamp(given_lastmod) + Page.new(url_by_locale: url_by_locale.to_h, lastmod:, modified_at: lastmod && Xml.time(lastmod), images: images_of(record), + rejected_lastmod: (given_lastmod if lastmod.nil? && given_lastmod.present?)) end def absolute(location) - location.start_with?("/") ? "#{configuration.host!}#{location}" : location + location = location.to_s + if location.start_with?("//") + location = "#{configuration.host!.split("//", 2).first}#{location}" + elsif location.start_with?("/") + location = "#{configuration.host!}#{location}" + end + encoded(location) + end + + # Route helpers encode their URLs already; a URL written by hand may hold spaces or accents. + def encoded(url) + return url if url.ascii_only? && !url.match?(UNSAFE_CHARACTER) + + url.b.gsub(UNSAFE_BYTE) { |byte| format("%%%02X", byte.ord) }.force_encoding(Encoding::UTF_8) end def lastmod_of(record) diff --git a/lib/mapitaine/tasks.rake b/lib/mapitaine/tasks.rake index 4714222..1fe25cf 100644 --- a/lib/mapitaine/tasks.rake +++ b/lib/mapitaine/tasks.rake @@ -17,7 +17,7 @@ namespace :mapitaine do report.call(Mapitaine.build(full: true)) end - desc "Render every sitemap without publishing it, and fail on duplicate, foreign or future-dated URLs" + desc "Render every sitemap without publishing it, and fail on what looks wrong in it" task check: :environment do warnings = Mapitaine.check abort warnings.join("\n") if warnings.any? diff --git a/lib/mapitaine/xml.rb b/lib/mapitaine/xml.rb index a84b442..0dc52d3 100644 --- a/lib/mapitaine/xml.rb +++ b/lib/mapitaine/xml.rb @@ -11,6 +11,9 @@ module Xml INDEX_CLOSE = "\n" # Google reads no more than 1,000 images per URL. MAX_IMAGES_PER_URL = 1_000 + # https://www.w3.org/TR/NOTE-datetime: a year, a month, a day, or a time with its zone. + W3C_DATE = /\A(\d{4})(?:-(\d{2})(?:-(\d{2}))?)?\z/ + W3C_TIME = /\A(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})(?::(\d{2})(?:\.\d+)?)?(Z|[+-]\d{2}:\d{2})\z/ module_function @@ -25,19 +28,49 @@ def sitemap_entry(url, lastmod) "#{escape(url)}#{lastmod_tag(lastmod)}\n" end + # The W3C date or time of a lastmod, in UTC, or nil when it is not one. def timestamp(value) - return if value.nil? - return value.utc.iso8601 if value.respond_to?(:utc) + case value + when nil then nil + when DateTime then value.to_time.getutc.iso8601 + when Date then value.iso8601 + when String then w3c_timestamp(value) + else value.getutc.iso8601 if value.respond_to?(:getutc) + end + end + + def w3c_timestamp(text) + if (date = W3C_DATE.match(text)) + year, month, day = date.captures + text if Date.valid_date?(year.to_i, (month || 1).to_i, (day || 1).to_i) + elsif (time = W3C_TIME.match(text)) + year, month, day, hour, minute, second, zone = time.captures + return unless Date.valid_date?(year.to_i, month.to_i, day.to_i) + + Time.new(year.to_i, month.to_i, day.to_i, hour.to_i, minute.to_i, second.to_i, (zone == "Z") ? "UTC" : zone).getutc.iso8601 + end + rescue ArgumentError + nil + end + + # A date alone is the day as it starts first, at UTC+14: no time zone has it in the future after that. + def time(stamp) + return Time.iso8601(stamp) if stamp.include?("T") + + year, month, day = stamp.split("-").map(&:to_i) + Time.new(year, month || 1, day || 1, 0, 0, 0, "+14:00") + end - value.respond_to?(:iso8601) ? value.iso8601 : value.to_s + # Rails locales spell regions with an underscore (pt_BR), hreflang with a hyphen (pt-BR). + def hreflang(locale) + locale.to_s.tr("_", "-") end def escape(text) text.to_s.encode(xml: :text).gsub('"', """) end - def lastmod_tag(value) - stamp = timestamp(value) + def lastmod_tag(stamp) stamp ? "#{escape(stamp)}" : "" end @@ -45,7 +78,8 @@ def alternates(page, x_default) return "" if page.url_by_locale.size < 2 links = page.url_by_locale.map { |locale, url| alternate(locale, url) } - links << alternate("x-default", page.url_by_locale[x_default]) if page.url_by_locale.key?(x_default) + default_url = page.url_by_locale[x_default] + links << alternate("x-default", default_url) if default_url links.join end diff --git a/test/build_test.rb b/test/build_test.rb index 4d1e7a9..ea4f82f 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -168,11 +168,66 @@ def test_special_characters_are_escaped_and_images_listed build xml = storage.xml("sitemap-post-00000.xml.gz") - assert_includes xml, "https://example.com/en/posts?title=Tom & "Jerry" <3" + assert_includes xml, "https://example.com/en/posts?title=Tom%20&%20%22Jerry%22%20%3C3" assert_includes xml, "https://example.com/images/1.jpg" \ "https://cdn.example.com/1.jpg?a=1&b=2" end + def test_a_url_written_by_hand_is_percent_encoded + create_post(1) + by_hand = Class.new(PostSitemap) do + url { |_post, locale| "/#{locale}/Café\u0001 d" + "\xE9j\xE0?q=100%25&off=100%" } + images { ["//cdn.example.com/a b.jpg"] } + end + Mapitaine.configuration.sources = [by_hand] + build + xml = storage.xml("sitemap-post-00000.xml.gz") + + assert_includes xml, "https://example.com/en/Caf%C3%A9%01%20d%E9j%E0?q=100%25&off=100%25" + assert_includes xml, "https://cdn.example.com/a%20b.jpg" + end + + def test_rails_locales_become_hreflang_codes_and_x_default_is_found_whatever_its_type + create_post(1, languages: "en,pt_BR") + strings = Class.new(PostSitemap) { locales { |post| post.languages.split(",") } } + Mapitaine.configuration.sources = [strings] + Mapitaine.configuration.x_default = :en + build + xml = storage.xml("sitemap-post-00000.xml.gz") + + assert_includes xml, %(hreflang="pt-BR" href="https://example.com/pt_BR/posts/1") + assert_includes xml, %(hreflang="x-default" href="https://example.com/en/posts/1") + end + + def test_two_languages_sharing_a_url_list_it_once + create_post(1, languages: "en,fr,de") + shared = Class.new(PostSitemap) { url { |post, locale| post_url(post, locale: (locale == :de) ? :en : locale) } } + Mapitaine.configuration.sources = [shared] + built = build + xml = storage.xml("sitemap-post-00000.xml.gz") + + assert_equal 2, built.url_count + assert_equal 2, xml.scan("").size + assert_includes xml, %(hreflang="de" href="https://example.com/en/posts/1") + end + + def test_lastmods_are_written_in_utc_and_the_index_takes_the_latest + create_post(1) + create_post(2) + create_post(3) + tokyo = Time.new(2026, 1, 15, 10, 0, 0, "+09:00") + stamps = {1 => "2026-01-15T10:00+09:00", 2 => "2026-01-15T05:00:00Z", 3 => "2026-02-30T05:00:00Z"} + zoned = Class.new(PostSitemap) { lastmod { |post| stamps.fetch(post.id) } } + Mapitaine.configuration.sources = [zoned, Class.new(PageSitemap) { lastmod { tokyo } }] + build + + assert_includes storage.xml("sitemap-post-00000.xml.gz"), "2026-01-15T01:00:00Z" + assert_includes storage.xml("sitemap-post-00000.xml.gz"), "https://example.com/en/posts/32026-01-15T05:00:00Z" + assert_includes storage.xml("sitemap-page.xml.gz"), "2026-01-15T01:00:00Z" + assert_equal 9 * 3600, tokyo.utc_offset + end + def test_a_lastmod_block_is_fingerprinted_by_the_expression_it_names create_post(1) reviewed = Class.new(PostSitemap) do diff --git a/test/check_test.rb b/test/check_test.rb index 4ba05f4..a4ae9c6 100644 --- a/test/check_test.rb +++ b/test/check_test.rb @@ -13,6 +13,7 @@ def test_a_sound_site_has_nothing_to_report_and_nothing_is_written def test_a_url_on_another_host_a_duplicate_and_a_future_lastmod_are_reported create_post(1, updated_at: Time.utc(2999, 1, 1)) create_post(12) + create_post(13, languages: "en") wrong = Class.new(PostSitemap) { url { |post, locale| (post.id == 1) ? "https://example.org/#{locale}" : "/same" } } Mapitaine.configuration.sources = [wrong] @@ -20,10 +21,62 @@ def test_a_url_on_another_host_a_duplicate_and_a_future_lastmod_are_reported "post: https://example.org/en is outside https://example.com", "post: https://example.org/fr is outside https://example.com", "post: https://example.org/en has a lastmod in the future (2999-01-01T00:00:00Z)", + "post: https://example.com/same is the URL of every language of its page: does the url block use the locale?", "post: https://example.com/same is listed twice" ], Mapitaine.check.map(&:to_s) end + def test_a_long_url_a_wrong_hreflang_and_a_lastmod_that_is_not_a_date_are_reported + create_post(1, title: "a" * 2_100) + wrong = Class.new(PostSitemap) do + url { |post, locale| "/#{locale}/#{post.title}" } + locales { %i[en english] } + lastmod { "yesterday" } + end + Mapitaine.configuration.sources = [wrong] + + assert_equal [ + "post: https://example.com/en/#{"a" * 2_100} is longer than 2048 characters", + "post: https://example.com/english/#{"a" * 2_100} is longer than 2048 characters", + %(post: https://example.com/english/#{"a" * 2_100} has hreflang "english", not a language code), + %(post: https://example.com/en/#{"a" * 2_100} has a lastmod that is not a W3C date ("yesterday"), left out) + ], Mapitaine.check.map(&:to_s) + end + + def test_mistakes_every_page_repeats_are_reported_once_and_impossible_dates_left_out + create_post(1) + create_post(2) + careless = Class.new(PostSitemap) do + url { |post| "/posts/#{post.id}" } + lastmod { "2026-13-01" } + end + Mapitaine.configuration.sources = [careless] + + assert_equal [ + "post: https://example.com/posts/1 is the URL of every language of its page: does the url block use the locale?", + %(post: https://example.com/posts/1 has a lastmod that is not a W3C date ("2026-13-01"), left out) + ], Mapitaine.check.map(&:to_s) + end + + def test_a_date_is_in_the_future_only_once_it_has_started_nowhere + dated = ->(date) { Class.new(PageSitemap) { lastmod { date } } } + first_date_on_earth = Time.now.getlocal("+14:00").to_date + Mapitaine.configuration.sources = [dated.call(first_date_on_earth)] + + assert_empty Mapitaine.check + Mapitaine.configuration.sources = [dated.call(first_date_on_earth + 1)] + + assert_equal 1, Mapitaine.check.size + end + + def test_one_wrong_hreflang_is_reported_once + create_post(1, languages: "en_GB,zz_top") + create_post(2, languages: "en_GB,zz_top") + Mapitaine.configuration.sources = [PostSitemap] + + assert_equal [%(post: https://example.com/zz_top/posts/1 has hreflang "zz-top", not a language code)], Mapitaine.check.map(&:to_s) + end + def test_a_build_reports_what_it_renders create_post(1, updated_at: Time.utc(2999, 1, 1)) diff --git a/test/source_test.rb b/test/source_test.rb index 0c0b7bd..7a6d215 100644 --- a/test/source_test.rb +++ b/test/source_test.rb @@ -44,7 +44,7 @@ def test_a_record_without_a_url_in_a_language_is_left_out_of_it english_only = Class.new(PostSitemap) { url { |post, locale| post_url(post, locale:) if locale == :en } } pages = english_only.new(Mapitaine.configuration).each_page("0").to_a - assert_equal [{en: "https://example.com/en/posts/1"}], pages.map(&:url_by_locale) + assert_equal [{"en" => "https://example.com/en/posts/1"}], pages.map(&:url_by_locale) end def test_route_helpers_use_the_configured_host_port_and_scheme