Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Error while searching Patient #1999

Closed
akshay-ap opened this issue Apr 20, 2020 · 12 comments · Fixed by #2012
Closed

Error while searching Patient #1999

akshay-ap opened this issue Apr 20, 2020 · 12 comments · Fixed by #2012
Assignees
Labels
🐛bug issue/pull request that documents/fixes a bug help wanted indicates that an issue is open for contributions patients issue/pull request that interacts with patients module
Milestone

Comments

@akshay-ap
Copy link
Contributor

🐛 Bug Report

Uncaught (in promise) SyntaxError: Invalid regular expression: /Adk\/: \ at end of pattern
    at RegExp (<anonymous>)
    at a.<anonymous> (PatientRepository.ts:21)
    at s (2.726afb2a.chunk.js?__WB_REVISION__=15200f807ae5907c400b:sourcemap:2)
    at Generator._invoke (2.726afb2a.chunk.js?__WB_REVISION__=15200f807ae5907c400b:sourcemap:2)
    at Generator.next (2.726afb2a.chunk.js?__WB_REVISION__=15200f807ae5907c400b:sourcemap:2)
    at r (2.726afb2a.chunk.js?__WB_REVISION__=15200f807ae5907c400b:sourcemap:2)
    at c (2.726afb2a.chunk.js?__WB_REVISION__=15200f807ae5907c400b:sourcemap:2)
    at 2.726afb2a.chunk.js?__WB_REVISION__=15200f807ae5907c400b:sourcemap:2
    at new Promise (<anonymous>)
    at a.<anonymous> (2.726afb2a.chunk.js?__WB_REVISION__=15200f807ae5907c400b:sourcemap:2)

A clear and concise description of what the bug is.

To Reproduce

  1. Create a patient
  2. Enter text like 'Adk/'(string containing '/') in patient search bar.

Expected behavior

No error.

Your Environment

  • node version: 12
  • fastify version: >=1.0.0
  • os: Windows
@jackcmeyer jackcmeyer added help wanted indicates that an issue is open for contributions patients issue/pull request that interacts with patients module 🐛bug issue/pull request that documents/fixes a bug labels Apr 21, 2020
@jackcmeyer jackcmeyer added this to the v2.0 milestone Apr 21, 2020
@jackcmeyer
Copy link
Member

Thanks for the report @akshay-ap. Confirmed that this is an issue. Seems like we should validate the characters in the search field.

@JDarke
Copy link
Contributor

JDarke commented Apr 23, 2020

I've got a fix that simply removes any backslashes, to avoid escaping the regexp. Does it need a wider-ranging check for other characters, or maybe just remove anything other than A-Z...?

PatientRepository.ts - Line 16 added, line 22 modified to replace 'text' with 'cleanText':

import shortid from 'shortid'
import Patient from '../../model/Patient'
import Repository from './Repository'
import { patients } from '../../config/pouchdb'

const formatPatientCode = (prefix: string, sequenceNumber: string) => `${prefix}${sequenceNumber}`

const getPatientCode = (): string => formatPatientCode('P-', shortid.generate())

export class PatientRepository extends Repository<Patient> {
  constructor() {
    super(patients)
  }

  async search(text: string): Promise<Patient[]> {
    const cleanText = text.replace(/\\/g, '')                   /* line added */
    return super.search({
      selector: {
        $or: [
          {
            fullName: {
              $regex: RegExp(cleanText, 'i'),                   /* line modified */
            },
          },
          {
            code: text,
          },
        ],
      },
    })
  }

  async save(entity: Patient): Promise<Patient> {
    const patientCode = getPatientCode()
    entity.code = patientCode
    return super.save(entity)
  }
}

export default new PatientRepository()

Either way, happy to take this one on if it's free.

@akshay-ap
Copy link
Contributor Author

The issue seems to be with string ending with any special characters in regex like [ or String having only * character.
Unhandled Rejection (SyntaxError): Invalid regular expression: /Test[/: Unterminated character class.
Unhandled Rejection (SyntaxError): Invalid regular expression: /*/: Nothing to repeat.
@JDarke, I would sugesst to find all the such cases rather than fixing the issue for just one case.

@JDarke
Copy link
Contributor

JDarke commented Apr 23, 2020

Ah, I thought it was just the backslash. I'll write a general case solution to remove anything that's not whitespace or alphabetical if Jack assigns me to the bug.

Is it better to fix as shown, by cleaning the search string, or by stopping the text field from accepting any disallowed characters in the first place?

@jackcmeyer
Copy link
Member

@JDarke I've assigned this to you.

@JDarke
Copy link
Contributor

JDarke commented Apr 23, 2020

Do international alphabetical characters need to be factored in to this, or is localisation being handled separately?

JDarke added a commit to JDarke/hospitalrun-frontend that referenced this issue Apr 23, 2020
Added a step to take the search string and remove all non-letter/whitespace characters. Line 16 and
22

fix HospitalRun#1999
JDarke added a commit to JDarke/hospitalrun-frontend that referenced this issue Apr 23, 2020
Added a step to remove all non-alphabetical and whitespace characters from search string regex

fix HospitalRun#1999
@jackcmeyer
Copy link
Member

international characters should be handled here. numbers, apostrophes, and other characters found in a name should also be supported.

@JDarke
Copy link
Contributor

JDarke commented Apr 23, 2020

Will do.

@JDarke
Copy link
Contributor

JDarke commented Apr 23, 2020

@akshay-ap ...I've run through numerous special characters, and so far my list of chars that break the search is as follows:

\ - Backslash
[  - Opening square bracket
 *  - Lone asterisk (with no preceding chars) 
** - Consecutive asterisks (with or without preceding chars) 
( ) - opening OR closing parentheses
? - Question mark

Does this cover all the instances you experienced?

@jackcmeyer
Copy link
Member

@JDarke I think that we can safely remove all of those characters you listed.

JDarke added a commit to JDarke/hospitalrun-frontend that referenced this issue Apr 24, 2020
Removed special chars that interfered with RegExp in search strings

re HospitalRun#1999
@JDarke
Copy link
Contributor

JDarke commented Apr 24, 2020

Ok, those are all removed in my last commit.

I think this should also be addressed at the Add New Patient input stage, too; currently it's possible to create a patient with a name containing those special characters, which the user will now not be able to accurately search for.

Separate issue to be raised, or to be included with this issue?

JDarke added a commit to JDarke/hospitalrun-frontend that referenced this issue Apr 25, 2020
Added a test to pass a search string including all prohibited chars, expects cleaned version of
string to be used for search.

re HospitalRun#1999
JDarke added a commit to JDarke/hospitalrun-frontend that referenced this issue Apr 25, 2020
Added test on the search string cleaning regex;  runs search string containing all prohibited chars,
expects cleaned version to be searched and matched

re HospitalRun#1999
JDarke added a commit to JDarke/hospitalrun-frontend that referenced this issue Apr 25, 2020
Removing mocha listed dependency added in error

re HospitalRun#1999
JDarke added a commit to JDarke/hospitalrun-frontend that referenced this issue Apr 29, 2020
Added escapeStringRegexp package to escape special chars in search string for p

fix HospitalRun#1999
@JDarke
Copy link
Contributor

JDarke commented Jun 17, 2020

I've just had this bug resurface

Can anyone else reproduce the error by entering a search string ending in backslash into the patient list search bar?

Screen Shot 2020-06-17 at 13 11 28
Screen Shot 2020-06-17 at 13 10 13

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐛bug issue/pull request that documents/fixes a bug help wanted indicates that an issue is open for contributions patients issue/pull request that interacts with patients module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants