Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #4440 - backslash escaped commas in search values #4609

Merged
merged 1 commit into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading