pozil/sfdc-ui-lookup-lwc

Fatal error on single asterisk

skycafemix opened this issue ยท 14 comments

Hello and thanks for your great work. I installed the latest version and get these errors.

Error 1. Fatal error on asterisk
Error 2. Non-Fatal error on two-letter strings that evaluate to one letter

Error 1. Fatal error on asterisk
Input:

*
*a

Result:
Pops up a "sorry to interrupt" error window with this info:

[Invalid regular expression: nothing to repeat]
RegExp()@[native code]
setSearchResults()
updateSearchTerm()
handleInput()

Patch:

I tried fixing it by adding asterisk into your REGEX_TRAP in various ways, but could not get it to work (tried 0, 1 and 2 backslashes but couldn't trap the *)

lookup.js line 14:
-const REGEX_TRAP = /[+\\?^${}()|[\]\\]/g;
+const REGEX_TRAP = /[*+\\?^${}()|[\]\\]/g;

Error 2. Non-Fatal error on two-letter strings that evaluate to one letter
SOSL requires 2 letter strings but lookup is passing through strings that evaluate to 1 letter.
Input:

\\
a?
"a

Ideally these should be caught in the lookup but I was able to fix them in the apex search method with the following code:

if (
            (searchTerm == '\\\\') || 
            (searchTerm.length() < 2) ||
            ((searchTerm.length() == 2) && (searchTerm.substring(1, 2) == '?')) ||
            (searchTerm.substring(0, 1) == '\"')
            ) {
            return new List<LookupSearchResult>();
}

I wonder is there a way to let us define a function that will clean input and pass it to the component in our connectedCallback? Then people could choose which letters to fix or maybe provide different cleaning logic.

pozil commented

Hi @skycafemix, thanks for reporting this issue. I can reproduce. I'm looking for a fix.

pozil commented

It took me some time to figure out but SOSL doesn't work well with special characters. I ended up filtering most of them on the JS side for the SOSL query and for the search result highlighter.
@skycafemix can you test the content of PR #80 and let me know if that works for you?

Sorry to keep you waiting, I will review but pls give me a few days as am on a deadline. Best regards

pozil commented

@skycafemix did you get the chance to test the fix?

pozil commented

@skycafemix last call to give me some feedback.

Sorry to keep you waiting. Still on the deadline and did not have time. I discovered an issue today that when using the lookup in a filter panel that can be collapsed and expanded. When collapsed the lookup's content is emptied. But I will test the asterisk update for you now.

I have a question.
This documentation says these characters must be escaped with a backslash for SOSL: ? & | ! { } [ ] ( ) ^ ~ * : \ " ' + -
Does replace(REGEX_SOSL_RESERVED, '.?') allow hyphen to be used?
One of my test data searches for CN-1234 and I think it works if you put quotes around it but I cannot tell if it will work with this code.

pozil commented

I read the documentation and my initial fix attempt was to do just that (escaping those characters). Unfortunately, SOSL doesn't behave well with these special characters so I ended up replacing them by ?. This avoids SOSL injection and prevents errors.
I ran some tests to search for accounts with dashes in their names and that worked well for me.

I see, then it should work okay. I am testing now and will let you know within an hour or so if I complete or run out of time now.

Hello Philippe, I am still getting this error.
I enter *a and apex still gives me:
FATAL_ERROR System.SearchException: search term must be longer than one character: ?a

I think you need a test case such as
const SAMPLE_SEARCH_TOO_SHORT_LEADING_STAR = '*a';

Since I was just deleting all asterisks in lookup.js this wasn't a problem, but your version sends ?* which still evaluates to a 1 character string apparently.

p.s. I think it would be useful to have tests against exactly the errors I reported:

*
*a
\\
a?
"a

As it is I "fixed" them with a combination of JS and Apex so it is a bit hard to un-noodle it to test.. But if you think it is working for you based on adding those tests I can test again or make a new LWC just to help. But I am kind of on a deadline now so cannot do more until then.

pozil commented

Thanks for catching that @skycafemix. I had it fixed at some point but it looks like I lost that fix after making some other changes while testing SOSL special characters.

I've restored that fix and added the relevant tests. Things should be sorted now.
Let me know if you want to test again but I feel confident enough to merge at this point.

Thanks, I trust you so we're good to go. Again many, many thanks for your work. It made two projects possible for me. One uses it for a one-time User lookup in a modal window, and the other for an Account search criteria input used in a search filters panel.

pozil commented

Sure thing, I'm glad that you could put it to good use :)

Fix included in release v2.9.1.