Send better user-agent values (and got config changes) (#7309)

* expose fetchLimitBytes/userAgent in got-config module

* export a function not a factory

* send better user-agent values

- add userAgentBase setting
- send short SHA in user agent on heroku
- set a version (tag or short SHA) in Dockefile and use
  it to report server version in UA for docker users

* add a comment explaining fileSize

Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com>

core/base-service/base.js

@@ -20,7 +20,7 @@ import {
   Deprecated,
 } from './errors.js'
 import { validateExample, transformExample } from './examples.js'
-import { fetchFactory } from './got.js'
+import { fetch } from './got.js'
 import {
   makeFullUrl,
   assertValidRoute,
@@ -432,7 +432,7 @@ class BaseService {
     },
     serviceConfig
   ) {
-    const { cacheHeaders: cacheHeaderConfig, fetchLimitBytes } = serviceConfig
+    const { cacheHeaders: cacheHeaderConfig } = serviceConfig
     const { regex, captureNames } = prepareRoute(this.route)
     const queryParams = getQueryParamNames(this.route)
 
@@ -441,8 +441,6 @@ class BaseService {
       ServiceClass: this,
     })
 
-    const fetcher = fetchFactory(fetchLimitBytes)
-
     camp.route(
       regex,
       handleRequest(cacheHeaderConfig, {
@@ -453,7 +451,7 @@ class BaseService {
           const namedParams = namedParamsForMatch(captureNames, match, this)
           const serviceData = await this.invoke(
             {
-              requestFetcher: fetcher,
+              requestFetcher: fetch,
               githubApiProvider,
               librariesIoApiProvider,
               metricHelper,

core/base-service/got-config.js

@@ -0,0 +1,26 @@
+import bytes from 'bytes'
+import configModule from 'config'
+import Joi from 'joi'
+import { fileSize } from '../../services/validators.js'
+
+const schema = Joi.object({
+  fetchLimit: fileSize,
+  userAgentBase: Joi.string().required(),
+}).required()
+const config = configModule.util.toObject()
+const publicConfig = Joi.attempt(config.public, schema, { allowUnknown: true })
+
+const fetchLimitBytes = bytes(publicConfig.fetchLimit)
+
+function getUserAgent(userAgentBase = publicConfig.userAgentBase) {
+  let version = 'dev'
+  if (process.env.DOCKER_SHIELDS_VERSION) {
+    version = process.env.DOCKER_SHIELDS_VERSION
+  }
+  if (process.env.HEROKU_SLUG_COMMIT) {
+    version = process.env.HEROKU_SLUG_COMMIT.substring(0, 7)
+  }
+  return `${userAgentBase}/${version}`
+}
+
+export { fetchLimitBytes, getUserAgent }

core/base-service/got-config.spec.js

@@ -0,0 +1,27 @@
+import { expect } from 'chai'
+import { getUserAgent } from './got-config.js'
+
+describe('getUserAgent function', function () {
+  afterEach(function () {
+    delete process.env.HEROKU_SLUG_COMMIT
+    delete process.env.DOCKER_SHIELDS_VERSION
+  })
+
+  it('uses the default userAgentBase', function () {
+    expect(getUserAgent()).to.equal('shields (self-hosted)/dev')
+  })
+
+  it('applies custom userAgentBase', function () {
+    expect(getUserAgent('custom')).to.equal('custom/dev')
+  })
+
+  it('uses short commit SHA from HEROKU_SLUG_COMMIT if available', function () {
+    process.env.HEROKU_SLUG_COMMIT = '92090bd44742a5fac03bcb117002088fc7485834'
+    expect(getUserAgent('custom')).to.equal('custom/92090bd')
+  })
+
+  it('uses short commit SHA from DOCKER_SHIELDS_VERSION if available', function () {
+    process.env.DOCKER_SHIELDS_VERSION = 'server-2021-11-22'
+    expect(getUserAgent('custom')).to.equal('custom/server-2021-11-22')
+  })
+})

core/base-service/got.js

@@ -1,7 +1,11 @@
 import got from 'got'
 import { Inaccessible, InvalidResponse } from './errors.js'
+import {
+  fetchLimitBytes as fetchLimitBytesDefault,
+  getUserAgent,
+} from './got-config.js'
 
-const userAgent = 'Shields.io/2003a'
+const userAgent = getUserAgent()
 
 async function sendRequest(gotWrapper, url, options) {
   const gotOptions = Object.assign({}, options)
@@ -22,8 +26,7 @@ async function sendRequest(gotWrapper, url, options) {
   }
 }
 
-const TEN_MB = 10485760
-function fetchFactory(fetchLimitBytes = TEN_MB) {
+function _fetchFactory(fetchLimitBytes = fetchLimitBytesDefault) {
   const gotWithLimit = got.extend({
     handlers: [
       (options, next) => {
@@ -52,4 +55,6 @@ function fetchFactory(fetchLimitBytes = TEN_MB) {
   return sendRequest.bind(sendRequest, gotWithLimit)
 }
 
-export { fetchFactory, userAgent }
+const fetch = _fetchFactory()
+
+export { fetch, _fetchFactory }

core/base-service/got.spec.js

@@ -1,6 +1,6 @@
 import { expect } from 'chai'
 import nock from 'nock'
-import { fetchFactory } from './got.js'
+import { _fetchFactory } from './got.js'
 import { Inaccessible, InvalidResponse } from './errors.js'
 
 describe('got wrapper', function () {
@@ -9,7 +9,7 @@ describe('got wrapper', function () {
       .get('/foo/bar')
       .once()
       .reply(200, 'x'.repeat(100))
-    const sendRequest = fetchFactory(100)
+    const sendRequest = _fetchFactory(100)
     const { res } = await sendRequest('https://www.google.com/foo/bar')
     expect(res.statusCode).to.equal(200)
   })
@@ -19,7 +19,7 @@ describe('got wrapper', function () {
       .get('/foo/bar')
       .once()
       .reply(200, 'x'.repeat(101))
-    const sendRequest = fetchFactory(100)
+    const sendRequest = _fetchFactory(100)
     return expect(
       sendRequest('https://www.google.com/foo/bar')
     ).to.be.rejectedWith(InvalidResponse, 'Maximum response size exceeded')
@@ -27,7 +27,7 @@ describe('got wrapper', function () {
 
   it('should throw an Inaccessible error if the request throws a (non-HTTP) error', async function () {
     nock('https://www.google.com').get('/foo/bar').replyWithError('oh no')
-    const sendRequest = fetchFactory(1024)
+    const sendRequest = _fetchFactory(1024)
     return expect(
       sendRequest('https://www.google.com/foo/bar')
     ).to.be.rejectedWith(Inaccessible, 'oh no')
@@ -36,7 +36,7 @@ describe('got wrapper', function () {
   it('should throw an Inaccessible error if the host can not be accessed', async function () {
     this.timeout(5000)
     nock.disableNetConnect()
-    const sendRequest = fetchFactory(1024)
+    const sendRequest = _fetchFactory(1024)
     return expect(
       sendRequest('https://www.google.com/foo/bar')
     ).to.be.rejectedWith(
@@ -49,14 +49,14 @@ describe('got wrapper', function () {
     nock('https://www.google.com', {
       reqheaders: {
         'user-agent': function (agent) {
-          return agent.startsWith('Shields.io')
+          return agent.startsWith('shields (self-hosted)')
         },
       },
     })
       .get('/foo/bar')
       .once()
       .reply(200)
-    const sendRequest = fetchFactory(1024)
+    const sendRequest = _fetchFactory(1024)
     await sendRequest('https://www.google.com/foo/bar')
   })
 

core/legacy/regular-update.js

@@ -3,9 +3,8 @@
  */
 
 import { InvalidResponse } from '../base-service/errors.js'
-import { fetchFactory } from '../../core/base-service/got.js'
+import { fetch } from '../../core/base-service/got.js'
 import checkErrorResponse from '../../core/base-service/check-error-response.js'
-const fetcher = fetchFactory()
 
 // Map from URL to { timestamp: last fetch time, data: data }.
 let regularUpdateCache = Object.create(null)
@@ -28,7 +27,7 @@ async function regularUpdate({
   json = true,
   scraper = buffer => buffer,
   options = {},
-  requestFetcher = fetcher,
+  requestFetcher = fetch,
 }) {
   const timestamp = Date.now()
   const cached = regularUpdateCache[url]

core/server/server.js

@@ -6,7 +6,6 @@ import path from 'path'
 import url, { fileURLToPath } from 'url'
 import { bootstrap } from 'global-agent'
 import cloudflareMiddleware from 'cloudflare-middleware'
-import bytes from 'bytes'
 import Camp from '@shields_io/camp'
 import originalJoi from 'joi'
 import makeBadge from '../../badge-maker/lib/make-badge.js'
@@ -18,7 +17,7 @@ import { makeSend } from '../base-service/legacy-result-sender.js'
 import { handleRequest } from '../base-service/legacy-request-handler.js'
 import { clearRegularUpdateCache } from '../legacy/regular-update.js'
 import { rasterRedirectUrl } from '../badge-urls/make-badge-url.js'
-import { nonNegativeInteger } from '../../services/validators.js'
+import { fileSize, nonNegativeInteger } from '../../services/validators.js'
 import log from './log.js'
 import PrometheusMetrics from './prometheus-metrics.js'
 import InfluxMetrics from './influx-metrics.js'
@@ -143,7 +142,8 @@ const publicConfigSchema = Joi.object({
   }).required(),
   cacheHeaders: { defaultCacheLengthSeconds: nonNegativeInteger },
   handleInternalErrors: Joi.boolean().required(),
-  fetchLimit: Joi.string().regex(/^[0-9]+(b|kb|mb|gb|tb)$/i),
+  fetchLimit: fileSize,
+  userAgentBase: Joi.string().required(),
   requestTimeoutSeconds: nonNegativeInteger,
   requestTimeoutMaxAgeSeconds: nonNegativeInteger,
   documentRoot: Joi.string().default(
@@ -433,7 +433,6 @@ class Server {
         {
           handleInternalErrors: config.public.handleInternalErrors,
           cacheHeaders: config.public.cacheHeaders,
-          fetchLimitBytes: bytes(config.public.fetchLimit),
           rasterUrl: config.public.rasterUrl,
           private: config.private,
           public: config.public,