diff --git a/src/Sanitize.php b/src/Sanitize.php index 819071391..7e88cb541 100644 --- a/src/Sanitize.php +++ b/src/Sanitize.php @@ -521,6 +521,10 @@ public function sanitize(string $data, int $type, string $base = '') } } + // Set the base before processing allowed nodes so `srcset` URLs + // can be absolutised, and keep only allowed HTML elements and + // attributes (this also rewrites ``/`` `srcset`). + $this->base = $base; if (!empty($this->allowed_html_elements_with_attributes)) { $this->enforce_allowed_html_nodes($document, $this->allow_data_attr, $this->allow_aria_attr); } @@ -547,7 +551,6 @@ public function sanitize(string $data, int $type, string $base = '') } // Replace relative URLs and blocks disallowed URI schemes (protocols) - $this->base = $base; foreach ($this->replace_url_attributes as $element => $attributes) { $this->replace_urls($document, $element, $attributes); } @@ -710,7 +713,8 @@ public function do_strip_htmltags(array $match) } /** - * Keep only allowed HTML elements (tags) and their allowed attributes. + * Keep only allowed HTML elements (tags) and their allowed attributes, + * and rewrite `srcset` URLs on `` and ``. */ protected function enforce_allowed_html_nodes(\DOMNode $element, bool $allow_data_attr = true, bool $allow_aria_attr = true): void { @@ -746,8 +750,8 @@ protected function enforce_allowed_html_nodes(\DOMNode $element, bool $allow_dat for ($i = $element->attributes->length - 1; $i >= 0; $i--) { $attr = $element->attributes[$i]->nodeName; // Skip data-*, aria-* if allowed - if (($allow_data_attr && str_starts_with($attr, 'data-')) - || ($allow_aria_attr && str_starts_with($attr, 'aria-'))) { + if (($allow_data_attr && strpos($attr, 'data-') === 0) + || ($allow_aria_attr && strpos($attr, 'aria-') === 0)) { continue; } @@ -755,6 +759,10 @@ protected function enforce_allowed_html_nodes(\DOMNode $element, bool $allow_dat $element->removeAttributeNode($element->attributes[$i]); } } + + if (in_array($tag, ['img', 'source'], true) && $element->hasAttribute('srcset')) { + $this->rewrite_img_srcset($element); + } } if ($element instanceof \DOMElement || $element instanceof \DOMDocument) { for ($i = $element->childNodes->length - 1; $i >= 0; $i--) { @@ -1004,6 +1012,158 @@ private function get_http_client(): Client return $this->http_client; } + + /** + * Give every URL in `srcset` the same treatment `replace_urls()` gives a + * single-URL attribute: absolutise against the document base, block a + * disallowed URI scheme, force HTTPS where configured. `srcset` holds a + * list rather than one URL, so it cannot go in `replace_url_attributes` + * and nothing else in the pipeline touches it. + * + * For `` only, when `src` is empty or a recognised placeholder + * (lazy-loading pattern), write the smallest `Nw` entry (or, failing + * that, the lowest-density `Nx` entry) as a fallback. The browser uses + * `srcset` for actual selection; `src` only loads when `srcset` can't be + * honoured (legacy browsers, non-browser API consumers), so the smallest + * is the safest fallback by bandwidth and is never larger than what + * `srcset` would have picked. This runs before `replace_urls()`, so that + * `src` still goes through the normal single-URL pass afterwards. + */ + private function rewrite_img_srcset(\DOMElement $element): void + { + $entries = $this->parse_srcset($element->getAttribute('srcset')); + if ($entries === []) { + return; + } + $absolutised = []; + foreach ($entries as $e) { + $abs = $this->registry->call(Misc::class, 'absolutize_url', [$e['url'], $this->base]); + if (!is_string($abs) || $abs === '') { + continue; + } + // Same condition as replace_urls(), but a blocked entry is dropped + // instead of being kept with an `unsafe:` prefix: an entry the + // browser must never pick has no reason to stay in the candidate + // list, and unlike `src` the attribute tolerates being shorter. + if ($this->disallowed_uri_schemes !== [] && !$this->is_allowed_scheme($abs)) { + continue; + } + $absolutised[] = ['url' => $this->https_url($abs), 'descriptor' => $e['descriptor'], 'w' => $e['w'], 'x' => $e['x']]; + } + if ($absolutised === []) { + $element->removeAttribute('srcset'); + return; + } + $element->setAttribute('srcset', implode(', ', array_map( + static function (array $e): string { + return $e['descriptor'] === '' ? $e['url'] : $e['url'] . ' ' . $e['descriptor']; + }, + $absolutised + ))); + + // `` (inside ``) has no `src` attribute; only `` + // needs a fallback `src` rewrite when its current value is a placeholder. + if ($element->tagName !== 'img') { + return; + } + $current = $element->getAttribute('src'); + if (!$this->is_placeholder_src($current)) { + return; + } + $candidates = array_values(array_filter($absolutised, static function (array $e): bool { + return $e['w'] > 0; + })); + if ($candidates !== []) { + usort($candidates, static function (array $a, array $b): int { + return $a['w'] <=> $b['w']; + }); + } else { + $candidates = array_values(array_filter($absolutised, static function (array $e): bool { + return $e['x'] > 0.0; + })); + usort($candidates, static function (array $a, array $b): int { + return $a['x'] <=> $b['x']; + }); + } + if ($candidates === []) { + return; + } + $element->setAttribute('src', $candidates[0]['url']); + } + + /** + * A `src` value is treated as a lazy-load placeholder if it is empty, + * matches the de-facto universal 1x1 transparent GIF marker (base64 + * encoding of the GIF89a header for a 1x1 transparent image), or is + * any other `data:` URI under 128 characters. The threshold sits + * between common placeholder sizes (~70-120 chars) and the smallest + * useful inline rasters (~200+ chars). + */ + private function is_placeholder_src(string $src): bool + { + if ($src === '') { + return true; + } + return strpos($src, 'data:') === 0 && + (strlen($src) < 128 || strpos($src, 'data:image/gif;base64,R0lGODlh') === 0); + } + + /** + * Follows the WHATWG srcset parsing algorithm rather than splitting on + * every comma: a URL is a run of non-whitespace, so unencoded commas + * inside URLs (Cloudinary, WordPress image CDNs) do not break entries + * apart. A comma only separates entries when it ends a URL or follows + * a descriptor. + * + * @return list + * `descriptor` is `''` when the source entry has no descriptor + * (browser implies `1x`, so `x` is 1.0); `w` is 0 and `x` is + * 0.0 when the descriptor is not of that kind. Both are used + * when picking a fallback `src`. + */ + private function parse_srcset(string $srcset): array + { + $out = []; + $len = strlen($srcset); + $pos = 0; + while ($pos < $len) { + // Skip whitespace and separating commas + while ($pos < $len && (ctype_space($srcset[$pos]) || $srcset[$pos] === ',')) { + $pos++; + } + $start = $pos; + while ($pos < $len && !ctype_space($srcset[$pos])) { + $pos++; + } + $url = (string) substr($srcset, $start, $pos - $start); + if ($url === '') { + break; + } + $descriptor = ''; + if (substr($url, -1) === ',') { + // Trailing comma terminates the entry: URL without descriptor + $url = rtrim($url, ','); + } else { + $end = strpos($srcset, ',', $pos); + if ($end === false) { + $end = $len; + } + $descriptor = trim((string) substr($srcset, $pos, $end - $pos)); + $pos = $end; + } + if ($url === '') { + continue; + } + $w = (preg_match('/^(\d+)w$/', $descriptor, $m) === 1) ? (int)$m[1] : 0; + if ($descriptor === '') { + $x = 1.0; + } else { + $x = (preg_match('/^(\d+(?:\.\d+)?)x$/', $descriptor, $m) === 1) ? (float)$m[1] : 0.0; + } + $out[] = ['url' => $url, 'descriptor' => $descriptor, 'w' => $w, 'x' => $x]; + } + return $out; + } } class_alias('SimplePie\Sanitize', 'SimplePie_Sanitize'); diff --git a/tests/Unit/SanitizeSrcsetTest.php b/tests/Unit/SanitizeSrcsetTest.php new file mode 100644 index 000000000..f3eff4521 --- /dev/null +++ b/tests/Unit/SanitizeSrcsetTest.php @@ -0,0 +1,255 @@ + $https_domains + */ + private function sanitize(string $html, string $base = 'https://example.com/', array $https_domains = []): string + { + $sanitize = new Sanitize(); + $sanitize->set_registry(new Registry()); + // Configure as SimplePie does, so srcset handling is exercised alongside + // the single-URL replace_urls() pass rather than in isolation. + $sanitize->set_url_replacements(); + $sanitize->set_https_domains($https_domains); + $sanitize->allowed_html_elements_with_attributes([ + 'picture' => [], + 'source' => ['type', 'src', 'srcset', 'sizes', 'media', 'height', 'width'], + 'img' => ['src', 'srcset', 'sizes', 'alt', 'width', 'height'], + ]); + + return $sanitize->sanitize($html, \SimplePie\SimplePie::CONSTRUCT_HTML, $base); + } + + public function testPicksSmallestSrcsetWidthWhenSrcIsDataUri(): void + { + // `src` is a fallback for clients that can't honour `srcset`; the smallest + // entry is the safest by bandwidth and never larger than the browser pick. + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="https://example.com/s.jpg"', $out); + self::assertStringNotContainsString('data:image/gif', $out); + } + + public function testRecognisesGifPlaceholderRegardlessOfLength(): void + { + // The R0lGODlh prefix is the de-facto universal 1x1 transparent GIF marker; + // treat it as a placeholder even if padded past the 128-char threshold. + $gif = 'data:image/gif;base64,R0lGODlh' . str_repeat('A', 200); + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="https://example.com/real.jpg"', $out); + self::assertStringNotContainsString('R0lGODlh', $out); + } + + public function testRetainsSrcsetAttribute(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('srcset=', $out); + self::assertStringContainsString('https://example.com/a.jpg 100w', $out); + self::assertStringContainsString('https://example.com/b.jpg 500w', $out); + } + + public function testRetainsSizesAttribute(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('sizes="(max-width: 800px) 100vw, 800px"', $out); + } + + public function testAbsolutisesRelativeSrcsetUrls(): void + { + $html = 'x'; + $out = $this->sanitize($html, 'https://example.com/articles/page'); + self::assertStringContainsString('https://example.com/img/a.jpg 100w', $out); + self::assertStringContainsString('https://example.com/img/b.jpg 500w', $out); + self::assertStringNotContainsString('srcset="/img/', $out); + } + + public function testEmptySrcWithBaseStillGetsSrcsetFallback(): void + { + // The allowed-node pass must run before `replace_urls()`. Otherwise an + // empty `src` is resolved to the document base first, the placeholder + // check no longer recognises it, and `src` is left pointing at the + // page the feed item came from instead of an image. + $html = 'x'; + $out = $this->sanitize($html, 'https://example.com/articles/page'); + self::assertStringContainsString('src="https://example.com/img/a.jpg"', $out); + self::assertStringNotContainsString('src="https://example.com/articles/page"', $out); + } + + public function testKeepsLegitimateImgSrc(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="https://example.com/real.jpg"', $out); + } + + public function testFallsBackToLowestDensityWhenNoWidthEntries(): void + { + // No Nw entries: clients that only read src would keep the placeholder, + // so fall back to the lowest-density entry instead. + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="https://example.com/a.jpg"', $out); + } + + public function testPrefersWidthOverDensityForFallbackSrc(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="https://example.com/w.jpg"', $out); + } + + public function testAbsolutisesDensityOnlySrcsetUrls(): void + { + $html = 'x'; + $out = $this->sanitize($html, 'https://example.com/page'); + self::assertStringContainsString('https://example.com/img/a.jpg 1x', $out); + self::assertStringContainsString('https://example.com/img/a@2x.jpg 2x', $out); + } + + public function testKeepsDensityEntriesWhenMixedWithWidthEntries(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('https://example.com/a.jpg 100w', $out); + self::assertStringContainsString('https://example.com/b.jpg 500w', $out); + self::assertStringContainsString('https://example.com/c@2x.jpg 2x', $out); + } + + public function testDoesNotDoubleEncodeAmpersandFromSrcset(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringNotContainsString('&amp;', $out); + } + + public function testNoOpOnImgWithoutSrcset(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="https://example.com/x.jpg"', $out); + } + + public function testPreservesBareSrcsetEntryWithoutDescriptor(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('srcset="https://example.com/a.jpg"', $out); + self::assertStringNotContainsString('https://example.com/a.jpg 1x', $out); + } + + public function testRewritesSourceSrcsetInPicture(): void + { + $html = '' + . '' + . 'x' + . ''; + $out = $this->sanitize($html, 'https://example.com/page'); + self::assertStringContainsString(' has no src attribute even if the img placeholder logic considered it. + // preg_match rather than a regex assertion: the assertion was renamed between + // the PHPUnit majors this package supports. + self::assertSame(0, preg_match('/]*\ssrc=/', $out)); + } + + public function testPreservesLongInlineBase64Src(): void + { + $big = 'data:image/png;base64,' . str_repeat('A', 300); + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="' . $big . '"', $out); + } + + public function testPreservesMidSizedInlineBase64Src(): void + { + // ~150 chars: comfortably larger than typical 1x1 placeholders but small + // enough that a loose threshold would have misclassified it. + $mid = 'data:image/png;base64,' . str_repeat('A', 150); + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('src="' . $mid . '"', $out); + } + + public function testKeepsCommasInsideSrcsetUrls(): void + { + // Cloudinary-style transformation URLs contain unencoded commas; + // splitting on every comma would corrupt them. + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('https://res.example.com/upload/w_300,c_scale/a.jpg 300w', $out); + self::assertStringContainsString('https://res.example.com/upload/w_600,c_scale/a.jpg 600w', $out); + self::assertStringContainsString('src="https://res.example.com/upload/w_300,c_scale/a.jpg"', $out); + } + + public function testParsesSrcsetEntriesSeparatedByCommaWithoutSpace(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringContainsString('https://example.com/a.jpg 100w', $out); + self::assertStringContainsString('https://example.com/b.jpg 500w', $out); + self::assertStringContainsString('src="https://example.com/a.jpg"', $out); + } + + public function testDropsDisallowedSchemesFromSrcset(): void + { + // srcset is never seen by replace_urls, so a disallowed scheme must be + // blocked here, and must not reach the src fallback either. + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringNotContainsString('javascript:', $out); + self::assertStringContainsString('src="https://example.com/a.jpg"', $out); + } + + public function testForcesHttpsInSrcsetAsWellAsSrc(): void + { + // replace_urls() upgrades src on a forced-HTTPS domain. srcset has to get + // the same upgrade here, otherwise the browser picks a candidate and loads + // it over http: mixed content on exactly the images this rewrite targets. + $html = 'x'; + $out = $this->sanitize($html, 'https://example.com/', ['cdn.example.com']); + self::assertStringContainsString('src="https://cdn.example.com/p.jpg"', $out); + self::assertStringContainsString('https://cdn.example.com/a.jpg 100w', $out); + self::assertStringContainsString('https://cdn.example.com/b.jpg 500w', $out); + self::assertStringNotContainsString('http://cdn.example.com', $out); + } + + public function testLeavesHttpSrcsetAloneOffForcedHttpsDomain(): void + { + $html = 'x'; + $out = $this->sanitize($html, 'https://example.com/', ['cdn.example.com']); + self::assertStringContainsString('http://other.example.net/a.jpg 100w', $out); + } + + public function testRemovesSrcsetWhenAllEntriesDisallowed(): void + { + $html = 'x'; + $out = $this->sanitize($html); + self::assertStringNotContainsString('srcset', $out); + self::assertStringContainsString('src="https://example.com/x.jpg"', $out); + } +}