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

',' in search param is mistreated as '\' #192

Closed
SherryH opened this Issue Jun 28, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@SherryH

SherryH commented Jun 28, 2015

In performing CarePlan search, if a comma ',' is contained in Token search param, the search result returns contains backslash '' instead if the search code is escaped according to FHIR spec. Empty list is returned if it is not escaped.

In string search without escaping, ',' is ignored and all the partial matching strings are returned.

jamesagnew added a commit that referenced this issue Jul 3, 2015

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Jul 3, 2015

Hi @SherryH ,

I'm not able to reproduce this so far, although I"m probably missing something. Would you be able to share a code sample illustrating the issue?

Cheers,
James

@SherryH

This comment has been minimized.

SherryH commented Jul 14, 2015

Hi @jamesagnew ,
Below is an extraction of the code. , is treated as __ when used at the end of the search parameter.

Thanks,
Sherry


@Search()
public List getResourcesByIdentifiers(
@RequiredParam(name = Practitioner.SP_IDENTIFIER) final TokenOrListParam theIdentifiers) {

// the special chars: ',', '' is not properly handled in extreme case
// e.g.
// -- RIGHT -- "Practitioner?identifier=system|code-include-but-not-end-with-comma,suffix" is solved as system = "system", and code= "code-include-but-not-end-with-comma,suffix"
// -- WRONG -- "Practitioner?identifier=system|code-end-with-comma," is solved as system = "system", and code= "code-end-with-comma"

}

jamesagnew added a commit that referenced this issue Jul 18, 2015

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Jul 18, 2015

Hi Sherry- I've added a unit test that tests exactly this, and it still seems to work fine (see af0db66 ).

Is it possible that there is an issue on the client end? E.g. "," is getting converted to ","? Are you using HAPI's client, or something else?

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Aug 5, 2015

Hi Sherry, after your comment on the last commit (thanks for that!) I was able to reproduce.

This is fixed, and in the process I also discovered that we also aren't always correctly handling escaped backslashes (foo\\bar should unescape to foo\bar) so this is fixed too. I'll push a git fix shortly, and there should be a new 1.2-SNAPSHOT build up in Maven within the next day or so.

Thanks again!!

@jamesagnew jamesagnew closed this in fd91ce7 Aug 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment