Skip to content

Commit

Permalink
HARMONY-1894: Rework label parameter handling and
Browse files Browse the repository at this point in the history
add maximum length validation
  • Loading branch information
indiejames committed Sep 27, 2024
1 parent 97401a7 commit 0c92c27
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 10 deletions.
2 changes: 1 addition & 1 deletion bin/stop-harmony-and-services
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion services/harmony/app/markdown/apis.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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 |
Expand Down
27 changes: 27 additions & 0 deletions services/harmony/app/middleware/label.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
// 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();
}
21 changes: 21 additions & 0 deletions services/harmony/app/middleware/parameter-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.)
Expand Down Expand Up @@ -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);
}
Expand Down
14 changes: 14 additions & 0 deletions services/harmony/app/models/label.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 3 additions & 5 deletions services/harmony/app/models/services/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -262,10 +261,9 @@ export default abstract class BaseService<ServiceParamType> {
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;
Expand Down
2 changes: 2 additions & 0 deletions services/harmony/app/routers/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down
22 changes: 22 additions & 0 deletions services/harmony/test/helpers/string.ts
Original file line number Diff line number Diff line change
@@ -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;
}
36 changes: 35 additions & 1 deletion services/harmony/test/models/label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
21 changes: 19 additions & 2 deletions services/harmony/test/parameters/label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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');
},
};

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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');
});
});
}
});

0 comments on commit 0c92c27

Please sign in to comment.