[Cases] Escape special characters in Parent issue field of Jira connector #145610

js-jankisalvi · 2022-11-17T18:41:32Z

Summary

Fixes #131281

Escapes special characters + - & | ! ( ) { } [ ] ^ ~ * ? \ : from parent issue field.

Before

After

Checklist

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

elasticmachine · 2022-11-17T18:41:35Z

Pinging @elastic/response-ops (Team:ResponseOps)

elasticmachine · 2022-11-17T18:41:35Z

Pinging @elastic/response-ops-cases (Feature:Cases)

…-ref HEAD~1..HEAD --fix'

jonathan-buttner · 2022-11-17T21:32:17Z

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.test.ts

+        configurationUtilities,
+        data: {
+          fields: {
+            summary: '[th!s^is()a-te+st-{~is*s|ue?or&and\\bye:}]',


hmm should the characters here be escaped?

it escapes only special characters, rest will be as it is. For above example the it will be: \\[th\\!s\\^is\$\$a\\-te\\+st\\-\\{\\~is\\*s\\|ue\\?or\\&and\\\\\\bye\\:\\}\\]

jonathan-buttner · 2022-11-17T21:34:19Z

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts

@@ -136,6 +136,9 @@ export const createExternalService = (
    }, '');
  };

+  const escapeSpecialCharacters = (str: string) =>


I know it's generally not an ideal solution to expose internal functions just for testing but could we do that here? Regex always makes me scared haha 😨 so it'd be good to have some tests around this function directly. It'll be way easier to test if we just export it from this file tool.

Could we also add a comment that includes this link: https://confluence.atlassian.com/jirasoftwareserver/search-syntax-for-text-fields-939938747.html

maybe something like:

These characters need to be escaped per Jira's search syntax, see for more details.

added comment in a73197

I know it's generally not an ideal solution to expose internal functions just for testing but could we do that here? Regex always makes me scared haha 😨 so it'd be good to have some tests around this function directly. It'll be way easier to test if we just export it from this file tool.

Shall I put this function in a x-pack/plugins/stack_connectors/server/connector_types/cases/jira/utils.ts for better access and tests?

fixed in f060450

jonathan-buttner · 2022-11-17T21:36:17Z

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts

@@ -136,6 +136,9 @@ export const createExternalService = (
    }, '');
  };

+  const escapeSpecialCharacters = (str: string) =>
+    str.replace(/[!^&*()+\-[\]\\/{}|:?~]/g, '\\\\$&')?.replace(/-/g, '\\\\x2d');


Do we need to escape unicode? I don't see it initially from searching this site: https://confluence.atlassian.com/jirasoftwareserver/search-syntax-for-text-fields-939938747.html

It'd be worth some manual testing though.

I don't think we need it. The encodeURIComponent will encode the URL correctly.

jonathan-buttner

For an additional improvement could we do a check for that particular error message that we're getting and transform it into something a little more useful for the user?

Maybe something like Search input is not formatted correctly and the include the jira error after that in the message.

cnasikas · 2022-11-18T08:24:39Z

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.test.ts

@@ -443,6 +443,70 @@ describe('Jira service', () => {
      });
    });

+    test('escapes special characters from summary', async () => {


This tests the createIncident function. We need to test the getIssues function. Something like this:

test('it should escape JQL special characters', async () => { requestMock.mockImplementation(() => createAxiosResponse({ data: { issues: issuesResponse, }, }) ); await service.getIssues('[th!s^is()a-te+st-{~is*s|ue?or&and\\bye:}]'); expect(requestMock).toHaveBeenLastCalledWith({ axios, logger, method: 'get', configurationUtilities, url: <the_url_escaped>, }); });

fixed in a73197

Thanks, Janki. I don't think we need the tests for the createIncident. We do not escape anything in the function neither we added any new logic.

cool, will remove it

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts

…com/js-jankisalvi/kibana into third-party-connector-special-chars

js-jankisalvi · 2022-11-18T15:40:23Z

For an additional improvement could we do a check for that particular error message that we're getting and transform it into something a little more useful for the user?

Maybe something like Search input is not formatted correctly and the include the jira error after that in the message.

Made a change in createErrorMessage function to handle error messages better in commit

So in this function we have 2 props of error response: const { errorMessages, errors } = errorResponse;
Turns out sometimes errors is null or empty but errorMessages has a descriptive error message. Which was not shown because our first condition was
if (errors == null) { return 'unknown: errorResponse.errors was null'; }

To reproduce this scenario, you can comment my escape character function and try putting some special characters on the Parent issue field:

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.test.ts

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts

…-fix'

…or-special-chars

…com/js-jankisalvi/kibana into third-party-connector-special-chars

…-ref HEAD~1..HEAD --fix'

…-fix'

…, removed double quotes

…-ref HEAD~1..HEAD --fix'

…com/js-jankisalvi/kibana into third-party-connector-special-chars

cnasikas

Thanks, Janki! Amazing job 🚀

kibana-ci · 2022-11-22T14:58:14Z

💚 Build Succeeded

Buildkite Build
Commit: 166a0ae

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	59	65	+6
`osquery`	109	115	+6
`securitySolution`	442	448	+6
total			+20

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	67	73	+6
`osquery`	110	117	+7
`securitySolution`	519	525	+6
total			+21

History

💚 Build #89908 succeeded 745ba12
💚 Build #89719 succeeded 3b31ab8
💚 Build #89422 succeeded 38a5bd7
💚 Build #89345 succeeded ba8686a
💚 Build #89298 succeeded bcf23eb
💔 Build #88961 failed 0390e56

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

cc @js-jankisalvi

…ctor (elastic#145610) ## Summary Fixes elastic#131281 Escapes special characters `+ - & | ! ( ) { } [ ] ^ ~ * ? \ :` from parent issue field. **Before** ![image](https://user-images.githubusercontent.com/117571355/202526389-a3428c44-45b5-498c-98af-4ca709ae6937.png) **After** ![image](https://user-images.githubusercontent.com/117571355/202525304-5023f27c-c3df-4839-8c5d-231dcd4a74e6.png) ![image](https://user-images.githubusercontent.com/117571355/202526111-4324ce5b-ea13-4bb6-8d96-388178b1b60d.png) ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit a42314d)

kibanamachine · 2022-11-22T15:11:01Z

💚 All backports created successfully

Status	Branch	Result
✅	8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

… connector (#145610) (#146009) # Backport This will backport the following commits from `main` to `8.6`: - [[Cases] Escape special characters in Parent issue field of Jira connector (#145610)](#145610)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Janki Salvi <[email protected]>

fix: escape special characters in Parent issue filed of stack connectors

516ab3b

js-jankisalvi added bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.6.0 labels Nov 17, 2022

js-jankisalvi self-assigned this Nov 17, 2022

js-jankisalvi requested a review from a team as a code owner November 17, 2022 18:41

js-jankisalvi requested a review from a team November 17, 2022 18:41

js-jankisalvi changed the title ~~fix: escape special characters in Parent issue filed of stack connectors~~ [Cases] escape special characters in Parent issue filed of stack connectors Nov 17, 2022

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

0390e56

…-ref HEAD~1..HEAD --fix'

cnasikas changed the title ~~[Cases] escape special characters in Parent issue filed of stack connectors~~ [Cases] escape special characters in Parent issue filed of Jira connector Nov 17, 2022

cnasikas changed the title ~~[Cases] escape special characters in Parent issue filed of Jira connector~~ [Cases] Escape special characters in Parent issue filed of Jira connector Nov 17, 2022

cnasikas added v8.7.0 release_note:skip Skip the PR/issue when compiling release notes labels Nov 17, 2022

jonathan-buttner reviewed Nov 17, 2022

View reviewed changes

cnasikas reviewed Nov 18, 2022

View reviewed changes

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts Outdated Show resolved Hide resolved

cnasikas reviewed Nov 18, 2022

View reviewed changes

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts Outdated Show resolved Hide resolved

cnasikas reviewed Nov 18, 2022

View reviewed changes

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts Outdated Show resolved Hide resolved

js-jankisalvi added 4 commits November 18, 2022 16:19

fix: review comment updates, tests added for getIssues

a73197e

Merge branch 'elastic:main' into third-party-connector-special-chars

0d4b60e

error messages improved

db5f500

Merge branch 'third-party-connector-special-chars' of https://github.…

d10e477

…com/js-jankisalvi/kibana into third-party-connector-special-chars

Merge branch 'main' into third-party-connector-special-chars

fa0e18e

cnasikas reviewed Nov 18, 2022

View reviewed changes

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.test.ts Outdated Show resolved Hide resolved

cnasikas reviewed Nov 18, 2022

View reviewed changes

x-pack/plugins/stack_connectors/server/connector_types/cases/jira/service.ts Outdated Show resolved Hide resolved

kibanamachine and others added 8 commits November 18, 2022 16:28

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

bcf23eb

…-fix'

Merge remote-tracking branch 'upstream/main' into third-party-connect…

585580e

…or-special-chars

Merge branch 'third-party-connector-special-chars' of https://github.…

ba8686a

…com/js-jankisalvi/kibana into third-party-connector-special-chars

updated test case to handle error message

78b037e

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

38a5bd7

…-ref HEAD~1..HEAD --fix'

fix: added regex function in utils file, added unit tests

f060450

Merge branch 'main' into third-party-connector-special-chars

464e8f1

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

3b31ab8

…-fix'

js-jankisalvi requested review from cnasikas and jonathan-buttner November 21, 2022 15:45

fix: remove test from createIncident

745ba12

peteharverson changed the title ~~[Cases] Escape special characters in Parent issue filed of Jira connector~~ [Cases] Escape special characters in Parent issue field of Jira connector Nov 22, 2022

js-jankisalvi and others added 5 commits November 22, 2022 14:29

fix: replaced single quote,and single backslash with four backslashes…

67d062d

…, removed double quotes

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

e747886

…-ref HEAD~1..HEAD --fix'

fix: updated service test

0c18cc9

Merge branch 'third-party-connector-special-chars' of https://github.…

3dd8f91

…com/js-jankisalvi/kibana into third-party-connector-special-chars

fix: removed double forward slash from regex

166a0ae

cnasikas approved these changes Nov 22, 2022

View reviewed changes

js-jankisalvi merged commit a42314d into elastic:main Nov 22, 2022

kibanamachine mentioned this pull request Nov 22, 2022

[8.6] [Cases] Escape special characters in Parent issue field of Jira connector (#145610) #146009

Merged

js-jankisalvi deleted the third-party-connector-special-chars branch January 4, 2023 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Escape special characters in Parent issue field of Jira connector #145610

[Cases] Escape special characters in Parent issue field of Jira connector #145610

js-jankisalvi commented Nov 17, 2022 •

edited by kibanamachine

Loading

elasticmachine commented Nov 17, 2022

elasticmachine commented Nov 17, 2022

jonathan-buttner Nov 17, 2022

js-jankisalvi Nov 18, 2022

jonathan-buttner Nov 17, 2022

jonathan-buttner Nov 17, 2022

js-jankisalvi Nov 18, 2022

js-jankisalvi Nov 21, 2022

jonathan-buttner Nov 21, 2022

js-jankisalvi Nov 21, 2022

jonathan-buttner Nov 17, 2022

cnasikas Nov 18, 2022

jonathan-buttner left a comment

cnasikas Nov 18, 2022

js-jankisalvi Nov 18, 2022

cnasikas Nov 18, 2022 •

edited

Loading

js-jankisalvi Nov 18, 2022

js-jankisalvi Nov 22, 2022

js-jankisalvi commented Nov 18, 2022 •

edited

Loading

cnasikas left a comment

kibana-ci commented Nov 22, 2022

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Nov 22, 2022

[Cases] Escape special characters in Parent issue field of Jira connector #145610

[Cases] Escape special characters in Parent issue field of Jira connector #145610

Conversation

js-jankisalvi commented Nov 17, 2022 • edited by kibanamachine Loading

Summary

Checklist

elasticmachine commented Nov 17, 2022

elasticmachine commented Nov 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathan-buttner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnasikas Nov 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

js-jankisalvi commented Nov 18, 2022 • edited Loading

cnasikas left a comment

Choose a reason for hiding this comment

kibana-ci commented Nov 22, 2022

💚 Build Succeeded

Metrics [docs]

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Nov 22, 2022

💚 All backports created successfully

Questions ?

js-jankisalvi commented Nov 17, 2022 •

edited by kibanamachine

Loading

cnasikas Nov 18, 2022 •

edited

Loading

js-jankisalvi commented Nov 18, 2022 •

edited

Loading