return error for invalid ranges

src/api/errors.ts

@@ -61,6 +61,12 @@ export const ParseError = createError(
   500
 )
 
+export const InvalidGlyphsRangeError = createError(
+  'FST_INVALID_GLYPHS_RANGE',
+  'Invalid range %s-%s',
+  400
+)
+
 export const createForwardedUpstreamError = (statusCode: number) =>
   createError(
     `FORWARDED_UPSTREAM_${statusCode}`,

src/api/glyphs.ts

@@ -1,10 +1,18 @@
 import fs from 'fs'
 import path from 'path'
 
-import { createStaticGlyphPath, SDF_STATIC_DIR } from '../lib/glyphs'
+import {
+  createStaticGlyphPath,
+  isValidGlyphsRange,
+  SDF_STATIC_DIR,
+} from '../lib/glyphs'
 import { isMapboxURL, normalizeGlyphsURL } from '../lib/mapbox_urls'
 import { Context } from '.'
-import { MBAccessTokenRequiredError, NotFoundError } from './errors'
+import {
+  InvalidGlyphsRangeError,
+  MBAccessTokenRequiredError,
+  NotFoundError,
+} from './errors'
 
 export type GlyphsResult =
   | { type: 'file'; data: string } // data is an absolute file path
@@ -44,6 +52,9 @@ function createGlyphsApi({ context }: { context: Context }): GlyphsApi {
   return {
     // TODO: Should we return always return the offline asset if it exists?
     async getGlyphs({ styleId, accessToken, font, start, end }) {
+      if (!isValidGlyphsRange(start, end))
+        throw new InvalidGlyphsRangeError(start, end)
+
       try {
         // 1. Attempt to get desired offline asset
         // Right now this is just a static asset bundled with the module.

src/lib/glyphs.ts

@@ -62,3 +62,18 @@ export function createStaticGlyphPath(
   const convertedFontName = font.replace(SPACES_REGEX, '-')
   return `${convertedFontName}/${start}-${end}.pbf`
 }
+
+const GLYPHS_RANGE_MULTIPLIER = 256
+const GLYPHS_RANGE_START_MAX = 65280
+
+// Based on start and end parameter docs here:
+// https://docs.mapbox.com/api/maps/fonts/#retrieve-font-glyph-ranges
+export function isValidGlyphsRange(start: number, end: number) {
+  if (start < 0 || end < 0) return false
+
+  const validStart =
+    start % GLYPHS_RANGE_MULTIPLIER === 0 && start <= GLYPHS_RANGE_START_MAX
+  const validEnd = end - start === GLYPHS_RANGE_MULTIPLIER - 1
+
+  return validStart && validEnd
+}

src/routes/fonts.ts

@@ -2,7 +2,11 @@ import { FastifyPluginAsync } from 'fastify'
 import { HTTPError } from 'got'
 import { Static, Type as T } from '@sinclair/typebox'
 
-import { NotFoundError, createForwardedUpstreamError } from '../api/errors'
+import {
+  NotFoundError,
+  createForwardedUpstreamError,
+  InvalidGlyphsRangeError,
+} from '../api/errors'
 import { DEFAULT_STATIC_FONT, createStaticGlyphPath } from '../lib/glyphs'
 
 const GetGlyphsParams = T.Object({

test/e2e/glyphs.test.js

@@ -76,7 +76,7 @@ test('GET /fonts/:fontstack/:start-:end.pbf sends fallback when specified font i
   t.equal(headers['content-type'], 'application/x-protobuf')
 })
 
-test('GET /fonts/:fontstack/:start-:end.pbf returns 404 for requests with non-existent ranges', async (t) => {
+test('GET /fonts/:fontstack/:start-:end.pbf returns 400 response for invalid glyph ranges', async (t) => {
   const server = createServer(t)
 
   const badStart = 1_000_000
@@ -86,7 +86,7 @@ test('GET /fonts/:fontstack/:start-:end.pbf returns 404 for requests with non-ex
     url: `/fonts/${createFontStack()}/${badStart}-${badStart + 255}.pbf`,
   })
 
-  t.equal(getGlyphsResponse.statusCode, 404)
+  t.equal(getGlyphsResponse.statusCode, 400)
 })
 
 test(

test/unit/glyphs.test.js

@@ -0,0 +1,61 @@
+const test = require('tape')
+const fs = require('fs')
+const path = require('path')
+
+const { isValidGlyphsRange } = require('../../dist/lib/glyphs')
+
+/**
+ * @param {number} factor
+ * @returns {[number, number]}
+ */
+function createRange(factor) {
+  if (factor < 0) throw new Error('Range factor must be >= 0')
+
+  const start = 256 * factor
+  const end = start + 255
+
+  return [start, end]
+}
+
+test('isValidGlyphsRange', (t) => {
+  t.test('returns false for any negative numbers', (st) => {
+    st.notOk(isValidGlyphsRange(-1, -1), 'when start and end are negative')
+    st.notOk(isValidGlyphsRange(-1, 1), 'when start is negative')
+    st.notOk(isValidGlyphsRange(1, -1), 'when end is negative')
+
+    st.end()
+  })
+
+  t.test('returns false for invalid positive values', (st) => {
+    st.notOk(isValidGlyphsRange(10, 255), 'invalid start')
+    st.notOk(isValidGlyphsRange(0, 10), 'invalid end')
+
+    st.end()
+  })
+
+  t.test(
+    'returns false for valid range that exceeds maximum range of 65280-65535',
+    (st) => {
+      const [start, end] = createRange(256)
+
+      st.notOk(isValidGlyphsRange(start, end), `${start}-${end} is too large`)
+
+      st.end()
+    }
+  )
+
+  t.test(
+    'returns true for valid ranges that do not exceed maximum range of 65280-65535',
+    (st) => {
+      const allValid = new Array(255).every((_, index) => {
+        return isValidGlyphsRange(createRange(index))
+      })
+
+      st.ok(allValid, 'all ranges from factor 0 to 255 are valid')
+
+      st.end()
+    }
+  )
+
+  t.end()
+})