diff --git a/bin/stop-harmony-and-services b/bin/stop-harmony-and-services index 6fb8d192e..9cfe3b9b8 100755 --- a/bin/stop-harmony-and-services +++ b/bin/stop-harmony-and-services @@ -2,7 +2,7 @@ # This script will delete all of the kubernetes harmony resources. If you use this script # you will also delete all of your local harmony jobs since the database will be destroyed. -# this will only have an effect if the development servies are running - otherwise it does +# this will only have an effect if the development services are running - otherwise it does # nothing bin/stop-dev-services diff --git a/services/harmony/app/markdown/apis.md b/services/harmony/app/markdown/apis.md index bf07a043c..cf720b767 100644 --- a/services/harmony/app/markdown/apis.md +++ b/services/harmony/app/markdown/apis.md @@ -66,7 +66,9 @@ As such it accepts parameters in the URL path as well as query parameters. | height | number of rows to return in the output coverage | | forceAsync | if "true", override the default API behavior and always treat the request as asynchronous | | format | the mime-type of the output format to return | -| label | the label(s) to add for the job that runs the request. Multiple labels can be specified as a comma-separated list. +| label | the label(s) to add for the job that runs the request. Multiple labels can be specified as a comma-separated list. A label can contain any characters up to a 255 +character limit, but if a label contains commas the request can only be a POST with +with the label field in the body. Labels are always converted to lower case. | maxResults | limits the number of input files processed in the request | | skipPreview | if "true", override the default API behavior and never auto-pause jobs | | ignoreErrors | if "true", continue processing a request to completion even if some items fail. If "false" immediately fail the request. Defaults to true | @@ -127,6 +129,8 @@ Currently only the `/position`, `/cube`, `/trajectory` and `/area` routes are su | ignoreErrors | if "true", continue processing a request to completion even if some items fail. If "false" immediately fail the request. Defaults to true | | interpolation | specify the interpolation method used during reprojection and scaling | | label | the label(s) to add for the job that runs the request. Multiple labels can be specified as a comma-separated list or as an array in the JSON body for POST requests. +A label can contain any characters up to a 255 character limit, but if a label +contains commas the request can only be a POST. Labels are always converted to lower case. | maxResults | limits the number of input files processed in the request | | scaleExtent | scale the resulting coverage along one axis to a given extent | | scaleSize | scale the resulting coverage along one axis to a given size | diff --git a/services/harmony/app/middleware/label.ts b/services/harmony/app/middleware/label.ts new file mode 100644 index 000000000..a47997a36 --- /dev/null +++ b/services/harmony/app/middleware/label.ts @@ -0,0 +1,27 @@ +import { Response, NextFunction } from 'express'; +import HarmonyRequest from '../models/harmony-request'; +import { parseMultiValueParameter } from '../util/parameter-parsing-helpers'; + +/** + * Express.js middleware to convert label parameter to an array (if needed) and add + * it to the body of the request + * + * @param req - The client request + * @param _res - The client response (not used) + * @param next - The next function in the middleware chain + */ +export default async function handleLabelParameter( + req: HarmonyRequest, _res: Response, next: NextFunction, +): Promise { + // Check if 'label' exists in the query parameters (GET), form-encoded body, or JSON body + let label = req.query.label || req.body.label; + + // If 'label' exists, convert it to an array (if not already) and assign it to 'label' in the body + if (label) { + label = parseMultiValueParameter(label); + req.body.label = label; + } + + // Call next to pass control to the next middleware or route handler + next(); +} \ No newline at end of file diff --git a/services/harmony/app/middleware/parameter-validation.ts b/services/harmony/app/middleware/parameter-validation.ts index b2c682a19..76354d7d0 100644 --- a/services/harmony/app/middleware/parameter-validation.ts +++ b/services/harmony/app/middleware/parameter-validation.ts @@ -9,6 +9,7 @@ import { getEdrParameters } from '../frontends/ogc-edr/index'; import env from '../util/env'; import { getRequestRoot } from '../util/url'; import { validateNoConflictingGridParameters } from '../util/grids'; +import { checkLabel } from '../models/label'; const { awsDefaultRegion } = env; @@ -154,6 +155,25 @@ function validateBucketPathParameter(req: HarmonyRequest): void { } } +/** + * Verify that the given label is valid. Send an error if it is not. + * @param req - The request object + * @throws RequestValidationError - if a label is invalid (too short or too long) + */ +export function validateLabelParameter(req: HarmonyRequest): void { + const labels = req.body.label; + + if (labels) { + for (const label of labels) { + const errMsg = checkLabel(label); + + if (errMsg) { + throw new RequestValidationError(errMsg); + } + } + } +} + /** * Validate that the parameter names are correct. * (Performs case insensitive comparison.) @@ -225,6 +245,7 @@ export default async function parameterValidation( if (req.url.match(EDR_ROUTE_REGEX)) { validateEdrParameterNames(req); } + validateLabelParameter(req); } catch (e) { return next(e); } diff --git a/services/harmony/app/models/label.ts b/services/harmony/app/models/label.ts index 3b331b44d..deefa6ebc 100644 --- a/services/harmony/app/models/label.ts +++ b/services/harmony/app/models/label.ts @@ -1,5 +1,19 @@ import { Transaction } from '../util/db'; +/** + * Returns an error message if a label is empty or exceeds 255 characters in length + * + * @param tag - The image tag to check + * @returns An error message if the tag is not valid, null otherwise + */ +export function checkLabel(label: string): string { + if (label.length < 1 || label.length > 255) { + const message = 'Labels must consist of at least one 1 and no more than 255 characters.'; + return message; + } + return null; +} + /** * Returns the labels for a given job * @param transaction - the transaction to use for querying diff --git a/services/harmony/app/models/services/base-service.ts b/services/harmony/app/models/services/base-service.ts index 05435bc7c..5c007e77d 100644 --- a/services/harmony/app/models/services/base-service.ts +++ b/services/harmony/app/models/services/base-service.ts @@ -18,7 +18,6 @@ import UserWork from '../user-work'; import { joinTexts } from '@harmony/util/string'; import { makeWorkScheduleRequest } from '../../backends/workflow-orchestration/work-item-polling'; import { QUERY_CMR_SERVICE_REGEX } from '../../backends/workflow-orchestration/util'; -import { parseMultiValueParameter } from '../../util/parameter-parsing-helpers'; export interface ServiceCapabilities { concatenation?: boolean; @@ -262,10 +261,9 @@ export default abstract class BaseService { logger.info('Invoking service for operation', { operation: this.operation }); // TODO handle the skipPreview parameter here when implementing HARMONY-1129 const job = this._createJob(getRequestUrl(req)); - const labels = req.query.label; - if (labels) { - job.labels = parseMultiValueParameter(labels as string); - } + const labels = req.body.label; + job.labels = labels || []; + await this._createAndSaveWorkflow(job); const { isAsync, jobID } = job; diff --git a/services/harmony/app/routers/router.ts b/services/harmony/app/routers/router.ts index 9aa1b8d01..e986ce8e1 100644 --- a/services/harmony/app/routers/router.ts +++ b/services/harmony/app/routers/router.ts @@ -41,6 +41,7 @@ import docsPage from '../frontends/docs/docs'; import { getCollectionCapabilitiesJson } from '../frontends/capabilities'; import extendDefault from '../middleware/extend'; import { getAdminHealth, getHealth } from '../frontends/health'; +import handleLabelParameter from '../middleware/label'; export interface RouterConfig { PORT?: string | number; // The port to run the frontend server on BACKEND_PORT?: string | number; // The port to run the backend server on @@ -199,6 +200,7 @@ export default function router({ skipEarthdataLogin = 'false' }: RouterConfig): next(new NotFoundError('Services can only be invoked when a valid collection is supplied in the URL path before the service name.')); }); result.use(logged(shapefileConverter)); + result.use(handleLabelParameter); result.use(logged(parameterValidation)); result.use(logged(parseGridMiddleware)); result.use(logged(preServiceConcatenationHandler)); diff --git a/services/harmony/test/helpers/string.ts b/services/harmony/test/helpers/string.ts new file mode 100644 index 000000000..be0b46b60 --- /dev/null +++ b/services/harmony/test/helpers/string.ts @@ -0,0 +1,22 @@ +/** + * Generate a random string of a given length + * + * @param length - the desired string length + * @param skipCodes - optional code points to skip, e.g., 0x002C for comma + * @returns a random string of the given length + */ +export function generateRandomString(length: number, skipCodes: number[] = []): string { + let result = ''; + + for (let i = 0; i < length; i++) { + // Generate a random Unicode code point from 0 to 65535 + let randomCodePoint: number; + do { + randomCodePoint = Math.floor(Math.random() * 65536); + } while (skipCodes.includes(randomCodePoint)); + + result += String.fromCharCode(randomCodePoint); + } + + return result; +} \ No newline at end of file diff --git a/services/harmony/test/models/label.ts b/services/harmony/test/models/label.ts index 1c9af70ff..c5d2b9553 100644 --- a/services/harmony/test/models/label.ts +++ b/services/harmony/test/models/label.ts @@ -3,7 +3,41 @@ import { describe, it } from 'mocha'; import { expect } from 'chai'; import { buildJob, getFirstJob } from '../helpers/jobs'; import { hookTransactionEach } from '../helpers/db'; -import { setLabelsForJob } from '../../app/models/label'; +import { checkLabel, setLabelsForJob } from '../../app/models/label'; + +// unit tests for `checkLabel` +describe('checkLabel', function () { + it('should return null for valid labels', function () { + // Examples of valid labels + const validLabels = [ + 'latest', + '1.0', + 'v1.๐Ÿ˜ฑ.1', + 'version_1.2.3', + 'a'.repeat(255), // Maximum length + ]; + + validLabels.forEach(label => { + const result = checkLabel(label); + expect(result).to.be.null; + }); + }); + + it('should return an error message for invalid labels', function () { + // Examples of invalid labels + const invalidLabels = [ + '', // empty + 'a'.repeat(256), // Exceeds maximum length + ]; + + const errorMessage = 'Labels must consist of at least one 1 and no more than 255 characters.'; + + invalidLabels.forEach(label => { + const result = checkLabel(label); + expect(result).to.equal(errorMessage); + }); + }); +}); describe('label CRUD', function () { hookTransactionEach(); diff --git a/services/harmony/test/parameters/label.ts b/services/harmony/test/parameters/label.ts index 47dc4fcef..4e6904699 100644 --- a/services/harmony/test/parameters/label.ts +++ b/services/harmony/test/parameters/label.ts @@ -7,6 +7,7 @@ import { hookRangesetRequest } from '../helpers/ogc-api-coverages'; import { hookRedirect } from '../helpers/hooks'; import { Job } from '../../app/models/job'; import { hookEdrRequest } from '../helpers/ogc-api-edr'; +import { generateRandomString } from '../helpers/string'; const collection = 'C1233800302-EEDTEST'; const granuleId = 'G1233800343-EEDTEST'; @@ -32,12 +33,10 @@ const hookPartials = { // eslint-disable-next-line @typescript-eslint/naming-convention 'OGC Coverages': (label: string[]): void => { hookRangesetRequest('1.0.0', collection, 'all', { query: { label }, username: 'joe' }); - hookRedirect('joe'); }, // eslint-disable-next-line @typescript-eslint/naming-convention 'OGC EDR (GET)': (label: string[]): void => { hookEdrRequest('position', '1.1.0', collection, { query: { ...edrQuery, label }, username: 'joe' }); - hookRedirect('joe'); }, }; @@ -50,6 +49,7 @@ describe('labels', function () { describe('when passing in a single label with the request', function () { hookPartials[apiType](['foo']); + hookRedirect('joe'); it('returns a 200 status code for the request', async function () { expect(this.res.status).to.equal(200); @@ -66,6 +66,7 @@ describe('labels', function () { describe('when passing in multiple labels with the request', function () { hookPartials[apiType](['bar', 'buzz']); + hookRedirect('joe'); it('returns a 200 status code for the request', async function () { expect(this.res.status).to.equal(200); @@ -81,6 +82,7 @@ describe('labels', function () { describe('when passing in mixed case labels with the request', function () { hookPartials[apiType](['Bar', 'buzz', 'bAZz']); + hookRedirect('joe'); it('returns a 200 status code for the request', async function () { expect(this.res.status).to.equal(200); @@ -96,6 +98,7 @@ describe('labels', function () { describe('when passing in labels with non-word characters with the request', function () { hookPartials[apiType](['b๐Ÿ™‚ar', 'bu#!zz']); + hookRedirect('joe'); it('returns a 200 status code for the request', async function () { expect(this.res.status).to.equal(200); @@ -107,5 +110,19 @@ describe('labels', function () { expect(job.job.labels).deep.equal(['b๐Ÿ™‚ar', 'bu#!zz']); }); }); + + describe('when passing in labels that are more than 255 characters long', async function () { + // a long label that does not contain commas + const veryLongLabel = generateRandomString(256, [0x002C]); + hookPartials[apiType](['b๐Ÿ™‚ar', veryLongLabel]); + + it('returns a 400 status code for the request', async function () { + expect(this.res.status).to.equal(400); + }); + + it('returns a meaningful error message', async function () { + expect(this.res.text).to.include('Labels must consist of at least one 1 and no more than 255 characters'); + }); + }); } }); \ No newline at end of file