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.

Sign up for GitHub

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

Jump to bottom

Interpreter conversion of string to number should throw on NaN #27788

Closed
w33ble opened this issue Dec 26, 2018 · 2 comments · Fixed by #50063
Closed

Interpreter conversion of string to number should throw on NaN #27788

w33ble opened this issue Dec 26, 2018 · 2 comments · Fixed by #50063
Assignees
@Dosant
Labels
bug Fixes for quality problems that affect the customer experience Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) good first issue low hanging fruit

Comments

@w33ble
Copy link
Contributor

w33ble commented Dec 26, 2018 •
edited
Loading

Kibana version: 6.6+

Describe the bug:

When converting from a string to a number, it simply passes the string through Number. This mostly works, except when the resulting value is NaN, in which case it just returns NaN, which is a number, but isn't valid.

Steps to reproduce:

  1. Set an number argument to a string value, like "cats" (font size on the plot function in Canvas is one example).
  2. The value of args passed into the function is the string "NaN"

Expected behavior:

The conversion should throw if isNaN(Number(value)) is true.
@w33ble w33ble added bug Fixes for quality problems that affect the customer experience triage_needed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Dec 26, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@w33ble w33ble changed the title Interpreter conversion of string to number should through on NaN Interpreter conversion of string to number should throw on NaN Dec 26, 2018
@w33ble w33ble mentioned this issue Dec 26, 2018
Merged
@timroes timroes added :AppArch and removed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Apr 17, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@lukeelmers lukeelmers added Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) good first issue low hanging fruit and removed triage_needed labels Apr 17, 2019
@Dosant Dosant self-assigned this Nov 6, 2019
@Dosant Dosant mentioned this issue Nov 6, 2019
Merged
7 tasks
Dosant added a commit that referenced this issue Nov 12, 2019
@Dosant
Bugfix: Interpreter conversion of string to number should throw on NaN …
a97c9d3 
…#27788 (#50063)
Dosant added a commit to Dosant/kibana that referenced this issue Nov 12, 2019
@Dosant
Bugfix: Interpreter conversion of string to number should throw on NaN …
d330b64 
…elastic#27788 (elastic#50063)
@Dosant Dosant mentioned this issue Nov 12, 2019
Merged
Dosant added a commit that referenced this issue Nov 12, 2019
@Dosant
Bugfix: Interpreter conversion of string to number should throw on NaN …
585f06b 
…#27788 (#50063) (#50287)
jloleysens added a commit to jloleysens/kibana that referenced this issue Nov 12, 2019
@jloleysens
Merge remote-tracking branch 'upstream/master' into test-1
37d4836 
* upstream/master:
  [ML] Make navigation in tests more stable (elastic#50132)
  Migrate authorization subsystem to the new platform.  (elastic#46145)
  Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063)
  Update dependency @elastic/charts to v14 (elastic#49947)
  [ML] Adding cloud specific ML node warning (elastic#50139)
  Fixing bugs in the Shareable Runtime (elastic#49965)
  Revert router base name for Uptime plugin to use hash in default path. (elastic#50095)
  Ability to have telemetry always opted in (elastic#49798)
  Add "Get Help" and "Kibana Feedback" links to the help popover (elastic#49797)
  Removes references to Elasticsearch mapping types (elastic#47610)
  [skip-ci] Replace coordinate map in Kibana getting started docs with Maps (elastic#50167)
  [ML] Indicate missing required privileges for import in File Data Viz (elastic#50147)
  [SIEM][Detection Engine] Removes technical debt and minor bug fixes (elastic#50111)
jloleysens added a commit to jloleysens/kibana that referenced this issue Nov 12, 2019
@jloleysens
Merge remote-tracking branch 'upstream/master' into test-3
7b512ad 
* upstream/master:
  Move table-list-view to kibana-react (elastic#50046)
  [ML] Stats bar for data frame analytics (elastic#49464)
  [ML] Make navigation in tests more stable (elastic#50132)
  Migrate authorization subsystem to the new platform.  (elastic#46145)
  Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063)
  Update dependency @elastic/charts to v14 (elastic#49947)
  [ML] Adding cloud specific ML node warning (elastic#50139)
  Fixing bugs in the Shareable Runtime (elastic#49965)
  Revert router base name for Uptime plugin to use hash in default path. (elastic#50095)
  Ability to have telemetry always opted in (elastic#49798)
  Add "Get Help" and "Kibana Feedback" links to the help popover (elastic#49797)
  Removes references to Elasticsearch mapping types (elastic#47610)
  [skip-ci] Replace coordinate map in Kibana getting started docs with Maps (elastic#50167)
  [ML] Indicate missing required privileges for import in File Data Viz (elastic#50147)
  [SIEM][Detection Engine] Removes technical debt and minor bug fixes (elastic#50111)
jloleysens added a commit to jloleysens/kibana that referenced this issue Nov 12, 2019
@jloleysens
Merge remote-tracking branch 'upstream/master' into np-migration/ua […
faa2676 
…skip ci]

* upstream/master:
  Move table-list-view to kibana-react (elastic#50046)
  [ML] Stats bar for data frame analytics (elastic#49464)
  [ML] Make navigation in tests more stable (elastic#50132)
  Migrate authorization subsystem to the new platform.  (elastic#46145)
  Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063)
  Update dependency @elastic/charts to v14 (elastic#49947)
  [ML] Adding cloud specific ML node warning (elastic#50139)
  Fixing bugs in the Shareable Runtime (elastic#49965)
  Revert router base name for Uptime plugin to use hash in default path. (elastic#50095)
  Ability to have telemetry always opted in (elastic#49798)
  Add "Get Help" and "Kibana Feedback" links to the help popover (elastic#49797)
  Removes references to Elasticsearch mapping types (elastic#47610)
  [skip-ci] Replace coordinate map in Kibana getting started docs with Maps (elastic#50167)
  [ML] Indicate missing required privileges for import in File Data Viz (elastic#50147)
  [SIEM][Detection Engine] Removes technical debt and minor bug fixes (elastic#50111)
jloleysens added a commit to jloleysens/kibana that referenced this issue Nov 13, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into fix/grokdebug…
0c102d9 
…ger-ace-theme

* 'master' of github.com:elastic/kibana: (56 commits)
  [ML] Server info service refactor (elastic#50302)
  Remove internal platform types exports (elastic#50427)
  [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312)
  [Telemetry] Server side fetcher (elastic#50015)
  [SIEM] Detection engine placeholders (elastic#50220)
  [Uptime] Donut chart loader position centered vertically  (elastic#50219)
  update telemetry banner notice text (elastic#50403)
  Fix aborting when searching without batching (elastic#49966)
  [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189)
  Revert chromedriver update (elastic#50324)
  Remove deprecated argument include_type_name from ES calls (elastic#50285)
  [Maps] add settings to maps telemetry (elastic#50161)
  remove visualize loader (elastic#46910)
  Fix misuse of react-router and react-router-dom (elastic#50120)
  Move table-list-view to kibana-react (elastic#50046)
  [ML] Stats bar for data frame analytics (elastic#49464)
  [ML] Make navigation in tests more stable (elastic#50132)
  Migrate authorization subsystem to the new platform.  (elastic#46145)
  Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063)
  Update dependency @elastic/charts to v14 (elastic#49947)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Dosant Dosant

Labels
bug Fixes for quality problems that affect the customer experience Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) good first issue low hanging fruit
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bugfix: Interpreter conversion of string to number should throw on NaN #27788 Dosant/kibana
5 participants
@w33ble @timroes @lukeelmers @Dosant @elasticmachine