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

[FIX] web: prevent crash when searching for escaped strings #29324

Closed
wants to merge 1 commit into
base: 12.0
from

Conversation

Projects
None yet
5 participants
@ged-odoo
Contributor

ged-odoo commented Dec 6, 2018

Recent works in the search view (in 12.0) on dynamic filters had an
unfortunate effect: when the user tries to input escaped strings (for
example "test" in custom filters, there was a crash.

The reason is that the JS python parser (or more specifically, the
tokenizer) was unable to parse that as a string. It worked before
because the web client sent the raw string to the server. However, with
dynamic filters, this is no longer the case, and we need to parse
domains to be able to combine them.

In this commit, we modify the tokenizer to be able to work with escaped
strings.

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@ged-odoo ged-odoo requested a review from Polymorphe57 Dec 6, 2018

@robodoo robodoo added the seen 🙂 label Dec 6, 2018

@ged-odoo

This comment has been minimized.

Contributor

ged-odoo commented Dec 6, 2018

@tbe-odoo You may be interested in this commit

@ged-odoo

This comment has been minimized.

Contributor

ged-odoo commented Dec 6, 2018

robodoo delegate=@Polymorphe57

@ged-odoo ged-odoo force-pushed the odoo-dev:12.0-fixes-ged branch from 7c3b179 to 3aed70c Dec 6, 2018

[FIX] web: prevent crash when searching for escaped strings
Recent works in the search view (in 12.0) on dynamic filters had an
unfortunate effect: when the user tries to input escaped strings (for
example "test" in custom filters, there was a crash.

The reason is that the JS python parser (or more specifically, the
tokenizer) was unable to parse that as a string.  It worked before
because the web client sent the raw string to the server.  However, with
dynamic filters, this is no longer the case, and we need to parse
domains to be able to combine them.

In this commit, we modify the tokenizer to be able to work with escaped
strings.

@ged-odoo ged-odoo force-pushed the odoo-dev:12.0-fixes-ged branch from 3aed70c to 1d14479 Dec 6, 2018

@Polymorphe57

This comment has been minimized.

Contributor

Polymorphe57 commented Dec 6, 2018

robodoo r+

@robodoo robodoo added the r+ 👌 label Dec 6, 2018

s = s
.replace(/\\\"/g, '\\u0022') // for double quote
.replace(/\\\'/g, '\\u0027'); // for single quote
var max=s.length, tokens = [], start, end;

This comment has been minimized.

@mge-odoo

mge-odoo Dec 6, 2018

Contributor

var max = s.length

@robodoo robodoo added the CI 🤖 label Dec 6, 2018

@C3POdoo C3POdoo added the RD label Dec 6, 2018

robodoo pushed a commit that referenced this pull request Dec 6, 2018

[FIX] web: prevent crash when searching for escaped strings
Recent works in the search view (in 12.0) on dynamic filters had an
unfortunate effect: when the user tries to input escaped strings (for
example "test" in custom filters, there was a crash.

The reason is that the JS python parser (or more specifically, the
tokenizer) was unable to parse that as a string.  It worked before
because the web client sent the raw string to the server.  However, with
dynamic filters, this is no longer the case, and we need to parse
domains to be able to combine them.

In this commit, we modify the tokenizer to be able to work with escaped
strings.

closes #29324
@robodoo

This comment has been minimized.

Contributor

robodoo commented Dec 6, 2018

Merged, thanks!

@robodoo robodoo closed this Dec 6, 2018

@KangOl KangOl deleted the odoo-dev:12.0-fixes-ged branch Dec 11, 2018

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