From 4c7007d0b6ac76a24d9bf7b2fc8dd5fb31cf0906 Mon Sep 17 00:00:00 2001
From: Kiko Beats <josefrancisco.verdu@gmail.com>
Date: Sun, 25 Feb 2024 17:02:07 +0100
Subject: [PATCH] fix(logo-favicon): favicon.ico with valid content-type

---
 .../metascraper-logo-favicon/package.json     |  1 +
 .../metascraper-logo-favicon/src/index.js     | 14 ++++--
 .../metascraper-logo-favicon/test/favicon.js  | 44 ++++++++++++++++++-
 .../metascraper-logo-favicon/test/helpers.js  | 24 ++++++++++
 4 files changed, 79 insertions(+), 4 deletions(-)
 create mode 100644 packages/metascraper-logo-favicon/test/helpers.js

diff --git a/packages/metascraper-logo-favicon/package.json b/packages/metascraper-logo-favicon/package.json
index ff3b7594b..5d5924027 100644
--- a/packages/metascraper-logo-favicon/package.json
+++ b/packages/metascraper-logo-favicon/package.json
@@ -30,6 +30,7 @@
     "reachable-url": "~1.8.0"
   },
   "devDependencies": {
+    "async-listen": "latest",
     "ava": "5"
   },
   "engines": {
diff --git a/packages/metascraper-logo-favicon/src/index.js b/packages/metascraper-logo-favicon/src/index.js
index 2df6804d0..5c92f837d 100644
--- a/packages/metascraper-logo-favicon/src/index.js
+++ b/packages/metascraper-logo-favicon/src/index.js
@@ -110,10 +110,18 @@ pickBiggerSize.sortBySize = collection =>
 const favicon = async (url, { gotOpts } = {}) => {
   const faviconUrl = logo('/favicon.ico', { url })
   if (!faviconUrl) return undefined
+
   const response = await reachableUrl(faviconUrl, gotOpts)
-  return reachableUrl.isReachable(response) &&
-    response.headers['content-type']?.startsWith('image')
-    ? faviconUrl
+  const contentType = response.headers['content-type']
+
+  const isValidContenType =
+    contentType &&
+    ['image/vnd.microsoft.icon', 'image/x-icon'].some(ct =>
+      contentType.includes(ct)
+    )
+
+  return isValidContenType && reachableUrl.isReachable(response)
+    ? response.url
     : undefined
 }
 
diff --git a/packages/metascraper-logo-favicon/test/favicon.js b/packages/metascraper-logo-favicon/test/favicon.js
index e426fc7aa..9f067d0ca 100644
--- a/packages/metascraper-logo-favicon/test/favicon.js
+++ b/packages/metascraper-logo-favicon/test/favicon.js
@@ -4,12 +4,54 @@ const test = require('ava')
 
 const { favicon } = require('..')
 
+const { runServer } = require('./helpers')
+
 test('return undefined if favicon is not reachable', async t => {
   const url = 'https://idontexist.lol'
   t.is(await favicon(url), undefined)
 })
 
-test("with { contentType: 'image/vnd.microsoft.icon' }", async t => {
+test("don't resolve favicon.ico with no content-type", async t => {
+  const server = await runServer(
+    t,
+    async ({ res }) => {
+      res.end('<svg></svg>')
+    },
+    { host: '0.0.0.0', port: 0 }
+  )
+  t.is(await favicon(server), undefined)
+})
+
+test("don't resolve favicon.ico with no valid content-type", async t => {
+  const server = await runServer(
+    t,
+    async ({ res }) => {
+      res.setHeader('content-type', 'image/svg+xml; charset=utf-8')
+      res.end('<svg></svg>')
+    },
+    { host: '0.0.0.0', port: 0 }
+  )
+  t.is(await favicon(server), undefined)
+})
+
+test("favicon.ico with 'image/vnd.microsoft.icon' content-type", async t => {
   const url = 'https://microlink.io/'
   t.is(await favicon(url), 'https://microlink.io/favicon.ico')
 })
+
+test("favicon.ico with 'image/x-icon' content-type", async t => {
+  const url = 'https://2miners.com/'
+  t.is(await favicon(url), 'https://2miners.com/favicon.ico')
+})
+
+test('handle redirects', async t => {
+  const server = await runServer(
+    t,
+    async ({ res }) => {
+      res.writeHead(301, { Location: 'https://microlink.io/favicon.ico' })
+      res.end()
+    },
+    { host: '0.0.0.0', port: 0 }
+  )
+  t.is(await favicon(server), 'https://microlink.io/favicon.ico')
+})
diff --git a/packages/metascraper-logo-favicon/test/helpers.js b/packages/metascraper-logo-favicon/test/helpers.js
new file mode 100644
index 000000000..9bf664f28
--- /dev/null
+++ b/packages/metascraper-logo-favicon/test/helpers.js
@@ -0,0 +1,24 @@
+'use strict'
+
+const { default: listen } = require('async-listen')
+const { createServer } = require('http')
+
+const closeServer = server =>
+  require('util').promisify(server.close.bind(server))()
+
+const runServer = async (t, handler, opts) => {
+  const server = createServer(async (req, res) => {
+    try {
+      await handler({ req, res })
+    } catch (error) {
+      console.error(error)
+      res.statusCode = 500
+      res.end()
+    }
+  })
+  const url = await listen(server, opts)
+  t.teardown(() => closeServer(server))
+  return url
+}
+
+module.exports = { runServer }