From fd593caafc0a7ffc2ce27ae2d649b6163c0c3f67 Mon Sep 17 00:00:00 2001 From: mikiher Date: Tue, 21 Oct 2025 09:38:35 +0300 Subject: [PATCH] SearchController: simplify query param validation logic --- server/controllers/SearchController.js | 212 ++++++++++--------------- server/utils/index.js | 49 ++++-- 2 files changed, 124 insertions(+), 137 deletions(-) diff --git a/server/controllers/SearchController.js b/server/controllers/SearchController.js index 72d215f34..9bb6e397d 100644 --- a/server/controllers/SearchController.js +++ b/server/controllers/SearchController.js @@ -4,7 +4,7 @@ const BookFinder = require('../finders/BookFinder') const PodcastFinder = require('../finders/PodcastFinder') const AuthorFinder = require('../finders/AuthorFinder') const Database = require('../Database') -const { isValidASIN, getQueryParamAsString } = require('../utils') +const { isValidASIN, getQueryParamAsString, ValidationError, NotFoundError } = require('../utils') // Provider name mappings for display purposes const providerMap = { @@ -39,73 +39,17 @@ class SearchController { constructor() {} /** - * Validates that multiple parameters are strings - * @param {Object} params - Object with param names as keys and values to validate - * @param {string} methodName - Name of the calling method for logging - * @returns {{valid: boolean, error?: {status: number, message: string}}} - */ - static validateStringParams(params, methodName) { - for (const [key, value] of Object.entries(params)) { - if (typeof value !== 'string') { - Logger.error(`[SearchController] ${methodName}: Invalid ${key} parameter`) - return { - valid: false, - error: { - status: 400, - message: 'Invalid request query params' - } - } - } - } - return { valid: true } - } - - /** - * Validates that a required string parameter exists and is a string - * @param {any} value - Value to validate - * @param {string} paramName - Parameter name for logging - * @param {string} methodName - Name of the calling method for logging - * @returns {{valid: boolean, error?: {status: number, message: string}}} - */ - static validateRequiredString(value, paramName, methodName) { - if (!value || typeof value !== 'string') { - Logger.error(`[SearchController] ${methodName}: Invalid or missing ${paramName}`) - return { - valid: false, - error: { - status: 400, - message: `Invalid or missing ${paramName}` - } - } - } - return { valid: true } - } - - /** - * Validates and fetches a library item by ID + * Fetches a library item by ID * @param {string} id - Library item ID * @param {string} methodName - Name of the calling method for logging - * @returns {Promise<{valid: boolean, libraryItem?: any, error?: {status: number, message: string}}>} + * @returns {Promise} */ - static async fetchAndValidateLibraryItem(id, methodName) { - const validation = SearchController.validateRequiredString(id, 'library item id', methodName) - if (!validation.valid) { - return validation - } - + static async fetchLibraryItem(id) { const libraryItem = await Database.libraryItemModel.getExpandedById(id) if (!libraryItem) { - Logger.error(`[SearchController] ${methodName}: Library item not found with id "${id}"`) - return { - valid: false, - error: { - status: 404, - message: 'Library item not found' - } - } + throw new NotFoundError(`library item "${id}" not found`) } - - return { valid: true, libraryItem } + return libraryItem } /** @@ -139,21 +83,25 @@ class SearchController { * @param {Response} res */ async findBooks(req, res) { - // Safely extract query parameters, rejecting arrays to prevent type confusion - const provider = getQueryParamAsString(req.query.provider, 'google') - const title = getQueryParamAsString(req.query.title, '') - const author = getQueryParamAsString(req.query.author, '') + try { + const query = req.query + const provider = getQueryParamAsString(query, 'provider', 'google') + const title = getQueryParamAsString(query, 'title', '') + const author = getQueryParamAsString(query, 'author', '') + const id = getQueryParamAsString(query, 'id', '', true) - // Validate string parameters - const validation = SearchController.validateStringParams({ provider, title, author }, 'findBooks') - if (!validation.valid) return res.status(validation.error.status).send(validation.error.message) + // Fetch library item + const libraryItem = await SearchController.fetchLibraryItem(id) - // Fetch and validate library item - const itemValidation = await SearchController.fetchAndValidateLibraryItem(req.query.id, 'findBooks') - if (!itemValidation.valid) return res.status(itemValidation.error.status).send(itemValidation.error.message) - - const results = await BookFinder.search(itemValidation.libraryItem, provider, title, author) - res.json(results) + const results = await BookFinder.search(libraryItem, provider, title, author) + res.json(results) + } catch (error) { + Logger.error(`[SearchController] findBooks: ${error.message}`) + if (error instanceof ValidationError || error instanceof NotFoundError) { + return res.status(error.status).send(error.message) + } + return res.status(500).send('Internal server error') + } } /** @@ -163,24 +111,24 @@ class SearchController { * @param {Response} res */ async findCovers(req, res) { - const query = req.query - const podcast = query.podcast === '1' || query.podcast === 1 - const title = getQueryParamAsString(query.title, '') - const author = getQueryParamAsString(query.author, '') - const provider = getQueryParamAsString(query.provider, 'google') + try { + const query = req.query + const podcast = query.podcast === '1' || query.podcast === 1 + const title = getQueryParamAsString(query, 'title', '', true) + const author = getQueryParamAsString(query, 'author', '') + const provider = getQueryParamAsString(query, 'provider', 'google') - // Validate required title - const titleValidation = SearchController.validateRequiredString(title, 'title', 'findCovers') - if (!titleValidation.valid) return res.status(titleValidation.error.status).send(titleValidation.error.message) - - // Validate other string parameters - const validation = SearchController.validateStringParams({ author, provider }, 'findCovers') - if (!validation.valid) return res.status(validation.error.status).send(validation.error.message) - - let results = null - if (podcast) results = await PodcastFinder.findCovers(title) - else results = await BookFinder.findCovers(provider, title, author) - res.json({ results }) + let results = null + if (podcast) results = await PodcastFinder.findCovers(title) + else results = await BookFinder.findCovers(provider, title, author) + res.json({ results }) + } catch (error) { + Logger.error(`[SearchController] findCovers: ${error.message}`) + if (error instanceof ValidationError) { + return res.status(error.status).send(error.message) + } + return res.status(500).send('Internal server error') + } } /** @@ -191,36 +139,42 @@ class SearchController { * @param {Response} res */ async findPodcasts(req, res) { - const term = getQueryParamAsString(req.query.term) - const country = getQueryParamAsString(req.query.country, 'us') + try { + const query = req.query + const term = getQueryParamAsString(query, 'term', '', true) + const country = getQueryParamAsString(query, 'country', 'us') - // Validate required term - const termValidation = SearchController.validateRequiredString(term, 'term', 'findPodcasts') - if (!termValidation.valid) return res.status(termValidation.error.status).send(termValidation.error.message) - - // Validate country parameter - const validation = SearchController.validateStringParams({ country }, 'findPodcasts') - if (!validation.valid) return res.status(validation.error.status).send(validation.error.message) - - const results = await PodcastFinder.search(term, { country }) - res.json(results) + const results = await PodcastFinder.search(term, { country }) + res.json(results) + } catch (error) { + Logger.error(`[SearchController] findPodcasts: ${error.message}`) + if (error instanceof ValidationError) { + return res.status(error.status).send(error.message) + } + return res.status(500).send('Internal server error') + } } /** * GET: /api/search/authors + * Note: This endpoint is not currently used in the web client. * * @param {RequestWithUser} req * @param {Response} res */ async findAuthor(req, res) { - const query = getQueryParamAsString(req.query.q) + try { + const query = getQueryParamAsString(req.query, 'q', '', true) - // Validate query parameter - const validation = SearchController.validateRequiredString(query, 'query', 'findAuthor') - if (!validation.valid) return res.status(validation.error.status).send(validation.error.message) - - const author = await AuthorFinder.findAuthorByName(query) - res.json(author) + const author = await AuthorFinder.findAuthorByName(query) + res.json(author) + } catch (error) { + Logger.error(`[SearchController] findAuthor: ${error.message}`) + if (error instanceof ValidationError) { + return res.status(error.status).send(error.message) + } + return res.status(500).send('Internal server error') + } } /** @@ -230,24 +184,30 @@ class SearchController { * @param {Response} res */ async findChapters(req, res) { - const asin = getQueryParamAsString(req.query.asin) - const region = getQueryParamAsString(req.query.region, 'us').toLowerCase() + try { + const query = req.query + const asin = getQueryParamAsString(query, 'asin', '', true) + const region = getQueryParamAsString(req.query.region, 'us').toLowerCase() - // Validate ASIN parameter - const asinValidation = SearchController.validateRequiredString(asin, 'asin', 'findChapters') - if (!asinValidation.valid) return res.json({ error: 'Invalid ASIN', stringKey: 'MessageInvalidAsin' }) + if (!isValidASIN(asin.toUpperCase())) throw new ValidationError('asin', 'is invalid') - if (!isValidASIN(asin.toUpperCase())) return res.json({ error: 'Invalid ASIN', stringKey: 'MessageInvalidAsin' }) - - // Validate region parameter - const validation = SearchController.validateStringParams({ region }, 'findChapters') - if (!validation.valid) res.json({ error: 'Invalid region', stringKey: 'MessageInvalidRegion' }) - - const chapterData = await BookFinder.findChapters(asin, region) - if (!chapterData) { - return res.json({ error: 'Chapters not found', stringKey: 'MessageChaptersNotFound' }) + const chapterData = await BookFinder.findChapters(asin, region) + if (!chapterData) { + return res.json({ error: 'Chapters not found', stringKey: 'MessageChaptersNotFound' }) + } + res.json(chapterData) + } catch (error) { + Logger.error(`[SearchController] findChapters: ${error.message}`) + if (error instanceof ValidationError) { + if (error.paramName === 'asin') { + return res.json({ error: 'Invalid ASIN', stringKey: 'MessageInvalidAsin' }) + } + if (error.paramName === 'region') { + return res.json({ error: 'Invalid region', stringKey: 'MessageInvalidRegion' }) + } + } + return res.status(500).send('Internal server error') } - res.json(chapterData) } /** diff --git a/server/utils/index.js b/server/utils/index.js index 0661d14f8..c7700a783 100644 --- a/server/utils/index.js +++ b/server/utils/index.js @@ -278,29 +278,56 @@ module.exports.timestampToSeconds = (timestamp) => { return null } +class ValidationError extends Error { + constructor(paramName, message, status = 400) { + super(`Query parameter "${paramName}" ${message}`) + this.name = 'ValidationError' + this.paramName = paramName + this.status = status + } +} +module.exports.ValidationError = ValidationError + +class NotFoundError extends Error { + constructor(message, status = 404) { + super(message) + this.name = 'NotFoundError' + this.status = status + } +} +module.exports.NotFoundError = NotFoundError + /** * Safely extracts a query parameter as a string, rejecting arrays to prevent type confusion * Express query parameters can be arrays if the same parameter appears multiple times * @example ?author=Smith => "Smith" - * @example ?author=Smith&author=Jones => null (array detected) + * @example ?author=Smith&author=Jones => throws error * - * @param {any} value - Query parameter value + * @param {Object} query - Query object + * @param {string} paramName - Parameter name * @param {string} defaultValue - Default value if undefined/null + * @param {boolean} required - Whether the parameter is required * @param {number} maxLength - Optional maximum length (defaults to 10000 to prevent ReDoS attacks) - * @returns {string|null} String value or null if invalid (array or too long) + * @returns {string} String value + * @throws {ValidationError} If value is an array + * @throws {ValidationError} If value is too long + * @throws {ValidationError} If value is required but not provided */ -module.exports.getQueryParamAsString = (value, defaultValue = '', maxLength = 1000) => { +module.exports.getQueryParamAsString = (query, paramName, defaultValue = '', required = false, maxLength = 1000) => { + const value = query[paramName] + if (value === undefined || value === null) { + if (required) { + throw new ValidationError(paramName, 'is required') + } + return defaultValue + } // Explicitly reject arrays to prevent type confusion if (Array.isArray(value)) { - return null - } - // Return default for undefined/null - if (value == null) { - return defaultValue + throw new ValidationError(paramName, 'is an array') } // Reject excessively long strings to prevent ReDoS attacks if (typeof value === 'string' && value.length > maxLength) { - return null + throw new ValidationError(paramName, 'is too long') } - return value + return String(value) }