[Ingest Pipelines] Fix serialization and deserialization of user input for "patterns" fields #94689

jloleysens · 2021-03-16T12:46:23Z

Summary

When users input text for the "pattern" field inside of the Grok, Dissect and Gsub processors we were not taking into account that this input comes from text area or text inputs where value is taken literally as it appears which is then JSON.stringify'd to something users do not expect. For instance, all backslash escaped chars (\t, \n) input by the user are encoded with an added backslash (\\t, \\n) resulting in a grok that matches for a backslash and a t char instead of a whitespace tab.

To fix this, when deserializing, we JSON.stringify the text we got from the server to escape whitespace and render a result in the text area/input that makes whitespaces visible.

When serializing the input we JSON.parse to remove one level of indentation so that we actually have \t instead of \\t as the text value in memory when the form is submitted.

Release note

We fixed a bug that incorrectly encoded user input for special characters like tabs (\t) and newlines (\n) for the Grok, Gsub and Dissect processors in the ingest pipelines editor.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

…b processors, also addded a test

elasticmachine · 2021-03-16T12:46:25Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

alisonelizabeth

Thanks for jumping on this bugfix so quickly!

I tested the original issue and confirmed it is now working as expected. However, I did find an issue when running the example in the dissect docs. I think we also need to account for escaping quotes. Can you take a look?

{
  "dissect": {
    "field": "message",
    "pattern" : "%{clientip} %{ident} %{auth} [%{@timestamp}] \"%{verb} %{request} HTTP/%{httpversion}\" %{status} %{size}"
   }
}

jloleysens · 2021-03-17T10:36:12Z

Very well caught @alisonelizabeth 👏🏻 !

I've pivoted the approach away from changing the indentation levels by hand and am rather making use of JSON.stringify when deserializing and JSON.parse when serializing to provide the functionality of escaping and unescaping input.

The quirk with this approach is that a user must provide a valid JSON string which requires an added level of validation on the input pattern. I think the strength with this is that users will be able to use all examples of patterns in the docs exactly as they appear in the input boxes for grok patterns.

Let me know what you think!

jloleysens · 2021-03-17T10:36:57Z

@elasticmachine merge upstream

…ialization

alisonelizabeth

Great work @jloleysens! Latest LGTM. Verified changes locally. Left one minor comment.

alisonelizabeth · 2021-03-17T16:46:37Z

...public/application/components/pipeline_editor/components/processor_form/processors/shared.ts

+  }
+};
+
+export const isJSONStringValidator: ValidationFunc = ({ value }) => {


Could you leverage https://github.com/elastic/kibana/blob/master/src/plugins/es_ui_shared/static/validators/string/is_json.ts here?

Yeah I looked at that validator, but the condition for returning true also requires that the result of JSON.parse be an object - in this case it will be a string.

kibana/src/plugins/es_ui_shared/static/validators/string/is_json.ts

Line 12 in 52a1ce1

if (parsedJSON && typeof parsedJSON !== 'object') {

For now I think it is OK to keep that behaviour and create our own validator.

…t for "patterns" fields (elastic#94689) * updated serialization and deserialization behavior of dissect and gsub processors, also addded a test * also fix grok processor * pivot input checking to use JSON.stringify and JSON.parse Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-03-18T10:23:22Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 1d0a4d7
Storybooks Preview
Pipeline Steps (look for red circles / failed steps)
Interpreting CI Failures

Failed CI Steps

Test Failures

Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/component_templates·ts.apis management index management Component templates Delete should return an error for any component templates not sucessfully deleted

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]       │
[00:00:00]         └-: apis
[00:00:00]           └-> "before all" hook in "apis"
[00:04:17]           └-: management
[00:04:17]             └-> "before all" hook in "management"
[00:04:36]             └-: index management
[00:04:36]               └-> "before all" hook in "index management"
[00:04:38]               └-: Component templates
[00:04:38]                 └-> "before all" hook in "Component templates"
[00:04:38]                 └-: Delete
[00:04:38]                   └-> "before all" hook for "should delete a component template"
[00:04:38]                   └-> "before all" hook for "should delete a component template"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_a]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_b]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_c]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_d]
[00:04:38]                   └-> should delete a component template
[00:04:38]                     └-> "before each" hook: global before each for "should delete a component template"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_a]
[00:04:38]                     └- ✓ pass  (33ms) "apis management index management Component templates Delete should delete a component template"
[00:04:38]                   └-> should delete multiple component templates
[00:04:38]                     └-> "before each" hook: global before each for "should delete multiple component templates"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_c]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_b]
[00:04:38]                     └- ✓ pass  (59ms) "apis management index management Component templates Delete should delete multiple component templates"
[00:04:38]                   └-> should return an error for any component templates not sucessfully deleted
[00:04:38]                     └-> "before each" hook: global before each for "should return an error for any component templates not sucessfully deleted"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_d]
[00:04:38]                     └- ✖ fail: apis management index management Component templates Delete should return an error for any component templates not sucessfully deleted
[00:04:38]                     │      Error: expected '[resource_not_found_exception] component_does_not_exist' to contain 'index_template_missing_exception'
[00:04:38]                     │       at Assertion.assert (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:100:11)
[00:04:38]                     │       at Assertion.contain (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:442:10)
[00:04:38]                     │       at Context.<anonymous> (test/api_integration/apis/management/index_management/component_templates.ts:362:45)
[00:04:38]                     │       at Object.apply (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/test/src/functional_test_runner/lib/mocha/wrap_function.js:73:16)
[00:04:38]                     │ 
[00:04:38]                     │

Stack Trace

Error: expected '[resource_not_found_exception] component_does_not_exist' to contain 'index_template_missing_exception'
    at Assertion.assert (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:100:11)
    at Assertion.contain (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:442:10)
    at Context.<anonymous> (test/api_integration/apis/management/index_management/component_templates.ts:362:45)
    at Object.apply (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/test/src/functional_test_runner/lib/mocha/wrap_function.js:73:16)

Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/component_templates·ts.apis management index management Component templates Delete should return an error for any component templates not sucessfully deleted

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 1 times on tracked branches: https://dryrun

[00:00:00]       │
[00:00:00]         └-: apis
[00:00:00]           └-> "before all" hook in "apis"
[00:04:18]           └-: management
[00:04:18]             └-> "before all" hook in "management"
[00:04:36]             └-: index management
[00:04:36]               └-> "before all" hook in "index management"
[00:04:38]               └-: Component templates
[00:04:38]                 └-> "before all" hook in "Component templates"
[00:04:38]                 └-: Delete
[00:04:38]                   └-> "before all" hook for "should delete a component template"
[00:04:38]                   └-> "before all" hook for "should delete a component template"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_a]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_b]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_c]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] adding component template [test_delete_component_template_d]
[00:04:38]                   └-> should delete a component template
[00:04:38]                     └-> "before each" hook: global before each for "should delete a component template"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_a]
[00:04:38]                     └- ✓ pass  (30ms) "apis management index management Component templates Delete should delete a component template"
[00:04:38]                   └-> should delete multiple component templates
[00:04:38]                     └-> "before each" hook: global before each for "should delete multiple component templates"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_c]
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_b]
[00:04:38]                     └- ✓ pass  (49ms) "apis management index management Component templates Delete should delete multiple component templates"
[00:04:38]                   └-> should return an error for any component templates not sucessfully deleted
[00:04:38]                     └-> "before each" hook: global before each for "should return an error for any component templates not sucessfully deleted"
[00:04:38]                     │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xxl-1616058523643195357] removing component template [test_delete_component_template_d]
[00:04:38]                     └- ✖ fail: apis management index management Component templates Delete should return an error for any component templates not sucessfully deleted
[00:04:38]                     │      Error: expected '[resource_not_found_exception] component_does_not_exist' to contain 'index_template_missing_exception'
[00:04:38]                     │       at Assertion.assert (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:100:11)
[00:04:38]                     │       at Assertion.contain (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:442:10)
[00:04:38]                     │       at Context.<anonymous> (test/api_integration/apis/management/index_management/component_templates.ts:362:45)
[00:04:38]                     │       at Object.apply (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/test/src/functional_test_runner/lib/mocha/wrap_function.js:73:16)
[00:04:38]                     │ 
[00:04:38]                     │

Stack Trace

Error: expected '[resource_not_found_exception] component_does_not_exist' to contain 'index_template_missing_exception'
    at Assertion.assert (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:100:11)
    at Assertion.contain (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/expect/expect.js:442:10)
    at Context.<anonymous> (test/api_integration/apis/management/index_management/component_templates.ts:362:45)
    at Object.apply (/dev/shm/workspace/parallel/10/kibana/node_modules/@kbn/test/src/functional_test_runner/lib/mocha/wrap_function.js:73:16)

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`ingestPipelines`	723.5KB	724.9KB	+1.4KB

History

💚 Build #113596 succeeded 1d0a4d7
💚 Build #113358 succeeded 60dac9c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…t for "patterns" fields (#94689) (#94897) * updated serialization and deserialization behavior of dissect and gsub processors, also addded a test * also fix grok processor * pivot input checking to use JSON.stringify and JSON.parse Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

…t for "patterns" fields (#94689) (#94896) * updated serialization and deserialization behavior of dissect and gsub processors, also addded a test * also fix grok processor * pivot input checking to use JSON.stringify and JSON.parse Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

updated serialization and deserialization behavior of dissect and gsu…

2b9f834

…b processors, also addded a test

jloleysens added release_note:fix v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Feature:Ingest Node Pipelines Ingest node pipelines management v7.13.0 v7.12.1 labels Mar 16, 2021

jloleysens requested a review from alisonelizabeth March 16, 2021 12:46

jloleysens requested a review from a team as a code owner March 16, 2021 12:46

also fix grok processor

60dac9c

alisonelizabeth reviewed Mar 16, 2021

View reviewed changes

pivot input checking to use JSON.stringify and JSON.parse

456fc5f

Merge branch 'master' into ingest-pipelines/fix-dissect-processor-ser…

1d0a4d7

…ialization

alisonelizabeth approved these changes Mar 17, 2021

View reviewed changes

jloleysens merged commit 628bb4b into elastic:master Mar 18, 2021

jloleysens deleted the ingest-pipelines/fix-dissect-processor-serialization branch March 18, 2021 09:07

This was referenced Mar 18, 2021

[7.x] [Ingest Pipelines] Fix serialization and deserialization of user input for "patterns" fields (#94689) #94896

Merged

[7.12] [Ingest Pipelines] Fix serialization and deserialization of user input for "patterns" fields (#94689) #94897

Merged

alisonelizabeth mentioned this pull request Jan 28, 2022

[Ingest Pipelines] Invalid error shown for grok processor #124027

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest Pipelines] Fix serialization and deserialization of user input for "patterns" fields #94689

[Ingest Pipelines] Fix serialization and deserialization of user input for "patterns" fields #94689

jloleysens commented Mar 16, 2021 •

edited

Loading

elasticmachine commented Mar 16, 2021

alisonelizabeth left a comment

jloleysens commented Mar 17, 2021

jloleysens commented Mar 17, 2021

alisonelizabeth left a comment

alisonelizabeth Mar 17, 2021

jloleysens Mar 18, 2021 •

edited

Loading

kibanamachine commented Mar 18, 2021 •

edited

Loading

Standard Out

Stack Trace

Standard Out

Stack Trace

[Ingest Pipelines] Fix serialization and deserialization of user input for "patterns" fields #94689

[Ingest Pipelines] Fix serialization and deserialization of user input for "patterns" fields #94689

Conversation

jloleysens commented Mar 16, 2021 • edited Loading

Summary

Release note

Checklist

elasticmachine commented Mar 16, 2021

alisonelizabeth left a comment

Choose a reason for hiding this comment

jloleysens commented Mar 17, 2021

jloleysens commented Mar 17, 2021

alisonelizabeth left a comment

Choose a reason for hiding this comment

alisonelizabeth Mar 17, 2021

Choose a reason for hiding this comment

jloleysens Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Mar 18, 2021 • edited Loading

💔 Build Failed

Failed CI Steps

Test Failures

Standard Out

Stack Trace

Standard Out

Stack Trace

Metrics [docs]

Async chunks

History

jloleysens commented Mar 16, 2021 •

edited

Loading

jloleysens Mar 18, 2021 •

edited

Loading

kibanamachine commented Mar 18, 2021 •

edited

Loading