Skip to content

Commit

Permalink
Fixes #4440 - backslash escaped commas in search values (#4609)
Browse files Browse the repository at this point in the history
  • Loading branch information
codyebberson committed May 31, 2024
1 parent 2ec6bc8 commit 0bab7dc
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 17 deletions.
8 changes: 4 additions & 4 deletions packages/core/src/search/match.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { CodeableConcept, Coding, Identifier, Reference, Resource, SearchParamet
import { evalFhirPath } from '../fhirpath/parse';
import { PropertyType, globalSchema } from '../types';
import { SearchParameterType, getSearchParameterDetails } from './details';
import { Filter, Operator, SearchRequest } from './search';
import { Filter, Operator, SearchRequest, splitSearchOnComma } from './search';

/**
* Determines if the resource matches the search request.
Expand Down Expand Up @@ -78,7 +78,7 @@ function matchesReferenceFilter(resource: Resource, filter: Filter, searchParam:
// Normalize the values array into reference strings
const references = values.map((value) => (typeof value === 'string' ? value : value.reference));

for (const filterValue of filter.value.split(',')) {
for (const filterValue of splitSearchOnComma(filter.value)) {
let match = references.includes(filterValue);
if (!match && filter.code === '_compartment') {
// Backwards compability for compartment search parameter
Expand Down Expand Up @@ -121,7 +121,7 @@ function matchesStringFilter(
const details = getSearchParameterDetails(resource.resourceType, searchParam);
const searchParamElementType = details.elementDefinitions?.[0].type?.[0].code;
const resourceValues = evalFhirPath(searchParam.expression as string, resource);
const filterValues = filter.value.split(',');
const filterValues = splitSearchOnComma(filter.value);
const negated = isNegated(filter.operator);
for (const resourceValue of resourceValues) {
for (const filterValue of filterValues) {
Expand Down Expand Up @@ -202,7 +202,7 @@ function matchesTokenCodeableConceptValue(

function matchesDateFilter(resource: Resource, filter: Filter, searchParam: SearchParameter): boolean {
const resourceValues = evalFhirPath(searchParam.expression as string, resource);
const filterValues = filter.value.split(',');
const filterValues = splitSearchOnComma(filter.value);
const negated = isNegated(filter.operator);
for (const resourceValue of resourceValues) {
for (const filterValue of filterValues) {
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/search/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
parseSearchRequest,
parseSearchUrl,
parseXFhirQuery,
splitSearchOnComma,
} from './search';

describe('Search Utils', () => {
Expand Down Expand Up @@ -424,4 +425,18 @@ describe('Search Utils', () => {
const actual = parseXFhirQuery(query, { '%patient': { type: 'Patient', value: patient } });
expect(actual).toEqual(expected);
});

test('Split search value on comma', () => {
expect(splitSearchOnComma('')).toEqual(['']);
expect(splitSearchOnComma('x')).toEqual(['x']);
expect(splitSearchOnComma('x,y')).toEqual(['x', 'y']);
expect(splitSearchOnComma('x,y,z')).toEqual(['x', 'y', 'z']);
expect(splitSearchOnComma('x,')).toEqual(['x', '']);
expect(splitSearchOnComma(',y')).toEqual(['', 'y']);
expect(splitSearchOnComma('x,,y')).toEqual(['x', '', 'y']);
expect(splitSearchOnComma('x\\,y')).toEqual(['x,y']);
expect(splitSearchOnComma('x\\,')).toEqual(['x,']);
expect(splitSearchOnComma('\\,y')).toEqual([',y']);
expect(splitSearchOnComma('x\\,,y')).toEqual(['x,', 'y']);
});
});
33 changes: 33 additions & 0 deletions packages/core/src/search/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -538,3 +538,36 @@ function formatIncludeTarget(kind: '_include' | '_revinclude', target: IncludeTa
kind + '=' + target.resourceType + ':' + target.searchParam + (target.targetType ? ':' + target.targetType : '')
);
}

/**
* Splits a FHIR search value on commas.
* Respects backslash escape.
*
* See: https://hl7.org/fhir/r4/search.html#escaping
*
* @param input - The FHIR search value to split.
* @returns The individual search values.
*/
export function splitSearchOnComma(input: string): string[] {
const result: string[] = [];
let current = '';
let escaped = false;

for (const c of input) {
if (escaped) {
current += c;
escaped = false;
} else if (c === '\\') {
escaped = true;
} else if (c === ',') {
result.push(current);
current = '';
} else {
current += c;
}
}

// Push the last segment
result.push(current);
return result;
}
4 changes: 2 additions & 2 deletions packages/server/src/fhir/lookups/lookuptable.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Operator as FhirOperator, Filter, SortRule } from '@medplum/core';
import { Operator as FhirOperator, Filter, SortRule, splitSearchOnComma } from '@medplum/core';
import { Resource, ResourceType, SearchParameter } from '@medplum/fhirtypes';
import { Pool, PoolClient } from 'pg';
import {
Expand Down Expand Up @@ -65,7 +65,7 @@ export abstract class LookupTable {
const columnName = this.getColumnName(filter.code);

const disjunction = new Disjunction([]);
for (const option of filter.value.split(',')) {
for (const option of splitSearchOnComma(filter.value)) {
if (filter.operator === FhirOperator.EXACT) {
disjunction.expressions.push(new Condition(new Column(lookupTableName, columnName), '=', option.trim()));
} else if (filter.operator === FhirOperator.CONTAINS) {
Expand Down
3 changes: 2 additions & 1 deletion packages/server/src/fhir/lookups/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
PropertyType,
SortRule,
splitN,
splitSearchOnComma,
toTypedValue,
TypedValue,
} from '@medplum/core';
Expand Down Expand Up @@ -413,7 +414,7 @@ function buildSimpleToken(
*/
function buildWhereExpression(tableName: string, filter: Filter): Expression | undefined {
const subExpressions = [];
for (const option of filter.value.split(',')) {
for (const option of splitSearchOnComma(filter.value)) {
const expression = buildWhereCondition(tableName, filter.operator, option);
if (expression) {
subExpressions.push(expression);
Expand Down
25 changes: 25 additions & 0 deletions packages/server/src/fhir/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
DiagnosticReport,
Encounter,
Goal,
Location,
MeasureReport,
Observation,
Organization,
Expand Down Expand Up @@ -793,6 +794,30 @@ describe('FHIR Search', () => {
expect((bundle2.entry?.[0]?.resource as StructureDefinition).name).toEqual('Questionnaire');
}));

test('String filter with escaped commas', async () =>
withTestContext(async () => {
// Create a name with commas
const name = randomUUID().replaceAll('-', ',');

const location = await repo.createResource<Location>({
resourceType: 'Location',
name,
});

const bundle = await repo.search<Location>({
resourceType: 'Location',
filters: [
{
code: 'name',
operator: Operator.EXACT,
value: name.replaceAll(',', '\\,'),
},
],
});
expect(bundle.entry?.length).toEqual(1);
expect(bundleContains(bundle, location)).toBe(true);
}));

test('Filter by _id', () =>
withTestContext(async () => {
// Unique family name to isolate the test
Expand Down
19 changes: 9 additions & 10 deletions packages/server/src/fhir/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
SearchRequest,
SortRule,
splitN,
splitSearchOnComma,
subsetResource,
toPeriod,
toTypedValue,
Expand Down Expand Up @@ -631,11 +632,11 @@ function buildNormalSearchFilterExpression(
} else if (filter.operator === Operator.PRESENT) {
return new Condition(new Column(table, details.columnName), filter.value === 'true' ? '!=' : '=', null);
} else if (param.type === 'string') {
return buildStringSearchFilter(table, details, filter.operator, filter.value.split(','));
return buildStringSearchFilter(table, details, filter.operator, splitSearchOnComma(filter.value));
} else if (param.type === 'token' || param.type === 'uri') {
return buildTokenSearchFilter(table, details, filter.operator, filter.value.split(','));
return buildTokenSearchFilter(table, details, filter.operator, splitSearchOnComma(filter.value));
} else if (param.type === 'reference') {
return buildReferenceSearchFilter(table, details, filter.value.split(','));
return buildReferenceSearchFilter(table, details, splitSearchOnComma(filter.value));
} else if (param.type === 'date') {
return buildDateSearchFilter(table, details, filter);
} else if (param.type === 'quantity') {
Expand All @@ -645,11 +646,9 @@ function buildNormalSearchFilterExpression(
filter.value
);
} else {
const values = filter.value
.split(',')
.map(
(v) => new Condition(new Column(undefined, details.columnName), fhirOperatorToSqlOperator(filter.operator), v)
);
const values = splitSearchOnComma(filter.value).map(
(v) => new Condition(new Column(undefined, details.columnName), fhirOperatorToSqlOperator(filter.operator), v)
);
const expr = new Disjunction(values);
return details.array ? new ArraySubquery(new Column(undefined, details.columnName), expr) : expr;
}
Expand Down Expand Up @@ -677,7 +676,7 @@ function trySpecialSearchParameter(
table,
{ columnName: 'id', type: SearchParameterType.UUID },
filter.operator,
filter.value.split(',')
splitSearchOnComma(filter.value)
);
case '_lastUpdated':
return buildDateSearchFilter(table, { type: SearchParameterType.DATETIME, columnName: 'lastUpdated' }, filter);
Expand All @@ -687,7 +686,7 @@ function trySpecialSearchParameter(
table,
{ columnName: 'compartments', type: SearchParameterType.UUID, array: true },
filter.operator,
filter.value.split(',')
splitSearchOnComma(filter.value)
);
case '_filter':
return buildFilterParameterExpression(selectQuery, resourceType, table, parseFilterParameter(filter.value));
Expand Down

0 comments on commit 0bab7dc

Please sign in to comment.