From e2352bc0871e511634b0c66a6179b250aca0f0e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 07:43:28 +0000 Subject: [PATCH 1/2] Percent-encode URLs, write hreflang codes, check more of each sitemap A URL built by hand could put a space, an accent or a control character in a file, which made it unreadable. Rails locales such as pt_BR went out as hreflang="pt_BR", which search engines ignore. A lastmod string was written whatever it held, and the index took the greatest string rather than the latest time. x_default was missed when the locales block returned strings, and two languages sharing a URL listed it twice. URLs are now percent-encoded outside RFC 3986's characters, locales are written with a hyphen, lastmods are read as W3C dates and written in UTC, and a page lists each of its URLs once. The check reports URLs over 2,048 characters, hreflang values that are not language codes, lastmods that are not dates, and an index with no file or more than 50,000. Assisted-by: Claude Code --- README.md | 20 ++++++++++++---- docs/guide.html | 15 +++++++++--- lib/mapitaine.rb | 2 ++ lib/mapitaine/builder.rb | 37 ++++++++++++++++++++++++++--- lib/mapitaine/page.rb | 12 +++++++--- lib/mapitaine/source.rb | 16 +++++++++++-- lib/mapitaine/tasks.rake | 2 +- lib/mapitaine/xml.rb | 34 +++++++++++++++++++++++---- test/build_test.rb | 50 +++++++++++++++++++++++++++++++++++++++- test/check_test.rb | 32 +++++++++++++++++++++++++ test/source_test.rb | 2 +- 11 files changed, 199 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 231e6b0..aae9588 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,14 @@ 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; +- 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 no file, or with more than the 50,000 files an index 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..db3c9c3 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..75d0e24 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_LENGTH = 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,7 @@ def build_in(directory, full:) @kept = [] @warnings = [] @changed_urls = [] + @checked_hreflangs = Set.new @configuration.sources.each { |source_class| build_source(source_class.new(@configuration)) } publish_index @@ -96,10 +103,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,13 +125,35 @@ 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_LENGTH} characters") if url.length > MAX_URL_LENGTH + 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 + inspect_lastmod(source, page) + end + + def inspect_lastmod(source, page) + return if page.lastmod.nil? + 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 + if modified_at.nil? + warn(source, page.urls.first, "has a lastmod that is not a W3C date (#{page.lastmod.inspect}), left out") + elsif modified_at > @manifest.built_at + warn(source, page.urls.first, "has a lastmod in the future (#{Xml.timestamp(page.lastmod)})") + end + end + + def inspect_index(url, file_count) + if file_count.zero? + warn("index", url, "lists no file: no source has a page") + elsif file_count > MAX_FILES_PER_INDEX + warn("index", url, "lists #{file_count} files, more than #{MAX_FILES_PER_INDEX}: raise shard_size") + end 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. diff --git a/lib/mapitaine/page.rb b/lib/mapitaine/page.rb index 3e9962c..2d987d6 100644 --- a/lib/mapitaine/page.rb +++ b/lib/mapitaine/page.rb @@ -1,14 +1,20 @@ # frozen_string_literal: true module Mapitaine - # One page of the site: the same content at one URL per language. + # One page of the site: the same content at one URL per language, keyed by hreflang. Page = Data.define(:url_by_locale, :lastmod, :images) do + # Two languages may share a URL; the file lists it once. def urls - url_by_locale.values + url_by_locale.values.uniq end def modified_at - lastmod.to_time if lastmod.respond_to?(:to_time) + return lastmod.getutc if lastmod.respond_to?(:getutc) && !lastmod.is_a?(Date) + + stamp = Xml.timestamp(lastmod) + return unless stamp + + stamp.include?("T") ? Time.iso8601(stamp) : Time.utc(*stamp.split("-").map(&:to_i)) end end end diff --git a/lib/mapitaine/source.rb b/lib/mapitaine/source.rb index 75dd4b2..b432506 100644 --- a/lib/mapitaine/source.rb +++ b/lib/mapitaine/source.rb @@ -16,6 +16,10 @@ 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_CHARACTER = /[^#{URL_CHARACTERS}]/ + UNSAFE_BYTE = Regexp.new("[^#{URL_CHARACTERS}]", Regexp::NOENCODING) class << self def inherited(subclass) @@ -144,13 +148,21 @@ 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)) end def absolute(location) - location.start_with?("/") ? "#{configuration.host!}#{location}" : location + location = location.to_s + encoded(location.start_with?("/") ? "#{configuration.host!}#{location}" : 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..d71d3c9 100644 --- a/lib/mapitaine/xml.rb +++ b/lib/mapitaine/xml.rb @@ -11,13 +11,16 @@ 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 # One per language. Each lists every language of the page, itself included: search engines drop # hreflang links that are not returned by the page they point to. def url_entries(page, x_default:) - shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, x_default)}#{images(page.images)}\n" + shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, hreflang(x_default))}#{images(page.images)}\n" page.urls.map { |url| "#{escape(url)}#{shared}" } end @@ -25,11 +28,31 @@ 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) + return text if text.match?(W3C_DATE) + + year, month, day, hour, minute, second, zone = W3C_TIME.match(text)&.captures + return unless year + + 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 + rescue ArgumentError + nil + 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) @@ -45,7 +68,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..03c24ad 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -168,11 +168,59 @@ 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) { url { |_post, locale| "/#{locale}/Café\u0001 d" + "\xE9j\xE0?q=100%25" } } + Mapitaine.configuration.sources = [by_hand] + build + + assert_includes storage.xml("sitemap-post-00000.xml.gz"), "https://example.com/en/Caf%C3%A9%01%20d%E9j%E0?q=100%25" + 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) + 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"} + 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.xml"), "sitemap-post-00000.xml.gz2026-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..fb9433f 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] @@ -24,6 +25,37 @@ def test_a_url_on_another_host_a_duplicate_and_a_future_lastmod_are_reported ], 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_an_empty_index_is_reported + Mapitaine.configuration.sources = [PostSitemap] + + assert_equal ["index: https://example.com/sitemap.xml lists no file: no source has a page"], Mapitaine.check.map(&:to_s) + 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 From 669c7178976b99a76e1d9d0558af314aa60d6057 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 07:47:46 +0000 Subject: [PATCH 2/2] Validate dates, catch locale-blind URLs, encode lone % and // URLs An impossible date such as 2026-13-01 crashed the build, and 2026-02-30 was written out. A date alone counted from UTC midnight, so today's date looked like the future east of UTC. A url block that forgot the locale no longer warned once its URLs were listed once per page. A protocol-relative URL got the host in front, and a % not followed by two hex digits stayed. Dates are validated, a date alone starts at UTC+14, a page whose languages share one URL is reported once per source, as is a lastmod that is not a date, and the lastmod is read once per page. An empty site no longer fails the check. Assisted-by: Claude Code --- README.md | 5 +++-- docs/guide.html | 4 ++-- lib/mapitaine/builder.rb | 38 +++++++++++++++++++++----------------- lib/mapitaine/page.rb | 15 +++------------ lib/mapitaine/source.rb | 17 +++++++++++++---- lib/mapitaine/xml.rb | 28 +++++++++++++++++++--------- test/build_test.rb | 13 ++++++++++--- test/check_test.rb | 27 ++++++++++++++++++++++++--- 8 files changed, 95 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index aae9588..d531d4f 100644 --- a/README.md +++ b/README.md @@ -214,10 +214,11 @@ bin/rails mapitaine:check 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; +- 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 no file, or with more than the 50,000 files an index may hold. +- an index with more than the 50,000 files it may hold. A build reports the same, for the files it rewrites, in `warnings`. diff --git a/docs/guide.html b/docs/guide.html index db3c9c3..b471b88 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -187,10 +187,10 @@

Checking

bin/rails mapitaine:check

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/builder.rb b/lib/mapitaine/builder.rb index 75d0e24..8437549 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -9,7 +9,7 @@ def changed? end # https://www.sitemaps.org/protocol.html - MAX_URL_LENGTH = 2_048 + 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 @@ -44,6 +44,8 @@ def build_in(directory, full:) @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 @@ -78,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 @@ -125,31 +127,34 @@ 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_LENGTH} characters") if url.length > MAX_URL_LENGTH + 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) - return if page.lastmod.nil? - - modified_at = page.modified_at - if modified_at.nil? - warn(source, page.urls.first, "has a lastmod that is not a W3C date (#{page.lastmod.inspect}), left out") - elsif modified_at > @manifest.built_at - warn(source, page.urls.first, "has a lastmod in the future (#{Xml.timestamp(page.lastmod)})") + 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 end def inspect_index(url, file_count) - if file_count.zero? - warn("index", url, "lists no file: no source has a page") - elsif file_count > MAX_FILES_PER_INDEX - warn("index", url, "lists #{file_count} files, more than #{MAX_FILES_PER_INDEX}: raise shard_size") - end + 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) @@ -158,8 +163,7 @@ def warn(source, url, message) # 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 2d987d6..ab62e43 100644 --- a/lib/mapitaine/page.rb +++ b/lib/mapitaine/page.rb @@ -1,20 +1,11 @@ # frozen_string_literal: true module Mapitaine - # One page of the site: the same content at one URL per language, keyed by hreflang. - Page = Data.define(:url_by_locale, :lastmod, :images) do - # Two languages may share a URL; the file lists it once. + # 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.uniq end - - def modified_at - return lastmod.getutc if lastmod.respond_to?(:getutc) && !lastmod.is_a?(Date) - - stamp = Xml.timestamp(lastmod) - return unless stamp - - stamp.include?("T") ? Time.iso8601(stamp) : Time.utc(*stamp.split("-").map(&:to_i)) - end end end diff --git a/lib/mapitaine/source.rb b/lib/mapitaine/source.rb index b432506..87c3365 100644 --- a/lib/mapitaine/source.rb +++ b/lib/mapitaine/source.rb @@ -18,8 +18,9 @@ class Source 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_CHARACTER = /[^#{URL_CHARACTERS}]/ - UNSAFE_BYTE = Regexp.new("[^#{URL_CHARACTERS}]", Regexp::NOENCODING) + 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) @@ -150,12 +151,20 @@ def page_for(record) location = instance_exec(record, locale, &definitions.fetch(:url)) [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 = location.to_s - encoded(location.start_with?("/") ? "#{configuration.host!}#{location}" : location) + 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. diff --git a/lib/mapitaine/xml.rb b/lib/mapitaine/xml.rb index d71d3c9..0dc52d3 100644 --- a/lib/mapitaine/xml.rb +++ b/lib/mapitaine/xml.rb @@ -12,7 +12,7 @@ module Xml # 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_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 @@ -20,7 +20,7 @@ module Xml # One per language. Each lists every language of the page, itself included: search engines drop # hreflang links that are not returned by the page they point to. def url_entries(page, x_default:) - shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, hreflang(x_default))}#{images(page.images)}\n" + shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, x_default)}#{images(page.images)}\n" page.urls.map { |url| "#{escape(url)}#{shared}" } end @@ -40,16 +40,27 @@ def timestamp(value) end def w3c_timestamp(text) - return text if text.match?(W3C_DATE) + 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) - year, month, day, hour, minute, second, zone = W3C_TIME.match(text)&.captures - return unless year - - 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 + 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 + # Rails locales spell regions with an underscore (pt_BR), hreflang with a hyphen (pt-BR). def hreflang(locale) locale.to_s.tr("_", "-") @@ -59,8 +70,7 @@ 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 diff --git a/test/build_test.rb b/test/build_test.rb index 03c24ad..ea4f82f 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -175,11 +175,16 @@ def test_special_characters_are_escaped_and_images_listed def test_a_url_written_by_hand_is_percent_encoded create_post(1) - by_hand = Class.new(PostSitemap) { url { |_post, locale| "/#{locale}/Café\u0001 d" + "\xE9j\xE0?q=100%25" } } + 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 storage.xml("sitemap-post-00000.xml.gz"), "https://example.com/en/Caf%C3%A9%01%20d%E9j%E0?q=100%25" + 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 @@ -209,13 +214,15 @@ def test_two_languages_sharing_a_url_list_it_once 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"} + 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 diff --git a/test/check_test.rb b/test/check_test.rb index fb9433f..a4ae9c6 100644 --- a/test/check_test.rb +++ b/test/check_test.rb @@ -21,6 +21,7 @@ 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 @@ -42,10 +43,30 @@ def test_a_long_url_a_wrong_hreflang_and_a_lastmod_that_is_not_a_date_are_report ], Mapitaine.check.map(&:to_s) end - def test_an_empty_index_is_reported - Mapitaine.configuration.sources = [PostSitemap] + 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 ["index: https://example.com/sitemap.xml lists no file: no source has a page"], Mapitaine.check.map(&:to_s) + assert_equal 1, Mapitaine.check.size end def test_one_wrong_hreflang_is_reported_once