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

[Kibana] SavedObject client mutates the KueryNode filter object #95784

Closed
cnasikas opened this issue Mar 30, 2021 · 2 comments · Fixed by #97081
Closed

[Kibana] SavedObject client mutates the KueryNode filter object #95784

cnasikas opened this issue Mar 30, 2021 · 2 comments · Fixed by #97081
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@cnasikas
Copy link
Member

cnasikas commented Mar 30, 2021

When you pass a KueryNode object as a filter to a method of the saved object’s API the object is being mutated by the API. This can lead to errors if the same filter is used in coming requests that use the same filter. Example:

const options = { filter: nodeBuilder.is('cases.attributes.status', 'open') };
const stats = await client.find({
        fields: [],
        page: 1,
        perPage: 1,
        sortField: defaultSortField,
        ...options,
        type: CASE_SAVED_OBJECT,
      });

return client.find({
        page: 1,
        perPage: stats.total,
        sortField: defaultSortField,
        ...options,
        type: CASE_SAVED_OBJECT,
      });

The filter will be nodeBuilder.is('cases.status', 'open') in the second call and will lead to error.

Using cloneDeep(options) make it work.

@cnasikas cnasikas added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Mar 30, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor

pgayvallet commented Mar 31, 2021

We do rewrite the filter's fields to convert from the SO attribute model to the underlying ES document fields.

When filter is passed as a string, we create the KueryNode internally, but when a real KueryNode is provided, the validation process mutates the input.

const filter = nodeBuilder.is('cases.attributes.status', 'open')
validateConvertFilterToKueryNode(['cases'], filter, mappings); // pass
validateConvertFilterToKueryNode(['cases'], filter, mappings); // fails

I guess we should internally deepClone the KueryNode when provided directly. I don't think it should have any significant performance impact.

export const validateConvertFilterToKueryNode = (
allowedTypes: string[],
filter: string | KueryNode,
indexMapping: IndexMapping
): KueryNode | undefined => {
if (filter && indexMapping) {
const filterKueryNode =
typeof filter === 'string' ? esKuery.fromKueryExpression(filter) : filter;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants