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

Typeahead highlight displays <strong> tag when empty spaces added at the end of query and does not escape html tags #4350

Open
pouellet opened this issue May 16, 2018 · 10 comments

Comments

@pouellet
Copy link

pouellet commented May 16, 2018

Bug description

This only happens when retrieving data asynchronously. We are using typeahead to trigger a quick search and display the three most relevant search results. We trim spaces around our search query, so a search with 'Ala ' (two trailing spaces) would return Alabama and Alaska. When this happens, the typeahead highlighting function mixes up the highlighting indexes and end up displaying stuff like trong>Alabama.

image

Similarly, if the data returned contains html, the html is not escaped. So if the backends return <script>Bermuda</script> and <i>Bern</i> when typing Be , then we end up with something like this:

image

Luckily, Angular detects the <script> tag as malicious and drops it, but it still result in messed up display and a few warnings in the console.

image

As a workaround, we are now wrapping the highlighting function with the following code:

    highlight(match: TypeaheadMatch, query: Array<String>): string {
        // Remove empty spaces from query. Entering 'Raster  ' in the search query results
        // in an array of ["Raster", " "] which messes up the highlighting.
        query = query.filter((value) => value.trim().length > 0);
        
        // Escape html from typeahead matches to avoid html injection and console warnings
        // when malicious tags get stripped out. It also messes up the highlighting.
        match = new TypeaheadMatch(match.item, escape(match.value), match.isHeader());
        return this.typeahead._container.hightlight(match, query);
    }

Plunker/StackBlitz that reproduces the issue

Extracted from the async demo, with a small change in the Regex to allow Ala to return Alabama. Also shows the issues with the html escape by typing Be. The second dropdown uses the workaround mentioned above to fix the issues.

https://stackblitz.com/edit/ngx-bootstrap-typeahead-highlight-bug

Versions of ngx-bootstrap, Angular, and Bootstrap

ngx-bootstrap: 2.0.5
Angular: 4.x, 5.x

@ghost
Copy link

ghost commented Sep 21, 2018

Ngx-bootstrap doesn't include this particular case. If you get an object with html tag, you should convert it by yourself. If a returned object doesn't consist string with html tag, everything will work fine

@pouellet
Copy link
Author

Ngx-bootstrap doesn't include this particular case. If you get an object with html tag, you should convert it by yourself.

Fair enough, thanks for the update.

If a returned object doesn't consist string with html tag, everything will work fine

The first example in my bug report (typing Ala followed by two white spaces) does not return any HTML tags, it's only returning a string (e.g. 'Alabama'). You can have a look at the StackBlitz repro.

@mykola-novytskyi
Copy link

What is the progress?

@frankfu03
Copy link

We have an angular web application which uses typeahead. One of the dropdown menus is a list of antenna-radome types that have fixed length of 20 characters and consist of two parts: antenna type and radome type. As radome type is always the last 4 chars, there are multiple spaces between antenna and radome types, which causes the same problem as pouellet described above.
Examples of the dropdown options:

AOA7490582.2    NONE
AOAD/M_B        DOME
AOAD/M_B        DUTD
AOAD/M_B        EMRA
AOAD/M_B        NONE
AOAD/M_TA_NGS   DOME

We currently use other third-party typeahead package which is not ideal. We are keen to use ngx-bootstrap/typeahead once this issue was fixed.

@UdayAppam
Copy link
Contributor

any update on this ??

@UdayAppam
Copy link
Contributor

#6434

Here I have tried the fix.
If any suggestions am open to discuss

@vibhus
Copy link

vibhus commented May 20, 2022

@valorkin there is a PR raised to fix this issue by @UdayAppam. Any plans to take this up?

@matthewfedak
Copy link

@valorkin my company would also like to see this fix used. Can offer a reward.

@ReaPer343
Copy link

@valorkin my company would also like to see this fix used. Can offer a reward.

I can fix that issue, how much reward I get

@matthewfedak
Copy link

@ReaPer343 Sorry, just seen this message. I already fixed it and have since changed companies. Thanks for the offer.

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

No branches or pull requests

8 participants