Fixed scripting issues due to improperly encoded kibana payload #7568

bevacqua · 2016-06-28T19:45:42Z

Screnshot:

spalger · 2016-06-28T20:27:13Z

src/ui/views/chrome.jade

@@ -12,5 +18,5 @@ html(lang='en')
    title Kibana
    block head
  body(kbn-chrome, id='#{appName}-body')
-    script window.__KBN__ = !{j(kibanaPayload)};


Can the !{} can just be replaced with #{} to use jade's default escaping?

No. That'd turn JSON into " and stuff. It HTML-encodes.

spalger · 2016-06-28T20:48:54Z

src/ui/views/chrome.jade

@@ -12,5 +12,5 @@ html(lang='en')
    title Kibana
    block head
  body(kbn-chrome, id='#{appName}-body')
-    script window.__KBN__ = !{j(kibanaPayload)};
+    meta(kbn-initial-state=JSON.stringify(kibanaPayload))


I like it, but I think we should use the meta tag how it's supposed to be use semantically:

in the <head>

with name="kbn-initial-state"

with content="..."

http://www.w3schools.com/tags/tag_meta.asp

spalger · 2016-06-28T20:56:34Z

lol

spalger · 2016-06-28T20:56:38Z

LGTM

bevacqua · 2016-06-28T20:57:58Z

@spalger That's exactly why I first went for the escaping approach. It's more naive but it doesn't quadruple the footprint of the state object.

epixa · 2016-06-28T21:00:40Z

Don't worry, our JS bundles are magnitudes larger than that. Just a drop in the bucket.

--------- **Commit 1:** [fix] Fixed scripting issues due to improperly encoded kibana payload. * Original sha: 08a6f40 * Authored by Nicolás Bevacqua <nicolasbevacqua@gmail.com> on 2016-06-28T19:44:18Z **Commit 2:** [fix] Switch to attribute-encoding * Original sha: 2cdb0f9 * Authored by Nicolás Bevacqua <nicolasbevacqua@gmail.com> on 2016-06-28T20:46:50Z **Commit 3:** [fix] Get rid of <meta> in favor of directive-style element. * Original sha: e33fa40 * Authored by Nicolás Bevacqua <nicolasbevacqua@gmail.com> on 2016-06-28T20:53:25Z

[backport] PR #7568 to 5.0.0-alpha4

This reverts commit 271aa69, reversing changes made to 1248b2f. This should not have been merged because tests did not pass.

…config-xss"" This reverts commit dda84e9.

--------- **Commit 1:** [fix] Tests passing. * Original sha: 41b0651 * Authored by Nicolás Bevacqua <nicolasbevacqua@gmail.com> on 2016-06-29T14:52:19Z **Commit 2:** Revert "Revert "Merge pull request #7568 from bevacqua/hotfix/config-xss"" This reverts commit dda84e9. * Original sha: 4854f2d * Authored by Nicolás Bevacqua <nicolasbevacqua@gmail.com> on 2016-06-29T15:02:17Z

Fixed scripting issues due to improperly encoded kibana payload Former-commit-id: 271aa69

This reverts commit 1785ccf [formerly 271aa69], reversing changes made to 7903281 [formerly 1248b2f]. This should not have been merged because tests did not pass. Former-commit-id: dda84e9

…config-xss"" This reverts commit b6ff2a6 [formerly dda84e9]. Former-commit-id: 4854f2d

`v93.3.0`⏩ `v93.4.0` --- ## [`v93.4.0`](https://github.com/elastic/eui/releases/v93.4.0) - Added the following properties to `EuiButtonGroup`'s `options` configs: `toolTipContent`, `toolTipProps`, and `title`. These new properties allow wrapping buttons in `EuiToolTips`, and additionally customizing or disabling the native browser `title` tooltip. ([#7461](elastic/eui#7461)) - Enhanced `EuiResizeObserver` and `useResizeObserver`'s performance to not trigger page reflows on resize event ([#7575](elastic/eui#7575)) - Updated `EuiSuperUpdateButton` to support custom button text via an optional `children` prop ([#7576](elastic/eui#7576)) **Bug fixes** - Fixed `EuiFlyout` to not repeatedly remove/add a body class on resize ([#7462](elastic/eui#7462)) - Fixed `EuiToast` title text to wrap instead of overflowing out of the container ([#7568](elastic/eui#7568)) - Fixed a visual bug with `EuiHeaderBreadcrumbs` with popovers ([#7580](elastic/eui#7580)) **Deprecations** - Deprecated `euiPalettePositive` and `euiPaletteNegative` in favour of a more culturally inclusive `euiPaletteGreen` and `euiPaletteRed` ([#7570](elastic/eui#7570)) - Deprecated all charts theme exports in favor of `@elastic/charts` exports: ([#7572](elastic/eui#7572)) - Deprecated `EUI_CHARTS_THEME_<DARK|LIGHT>` in favor of `<DARK|LIGHT>_THEME` from `@elastic/charts`. ([#7572](elastic/eui#7572)) - Deprecated `EUI_SPARKLINE_THEME_PARTIAL` in favor of `useSparklineOverrides` theme from the kibana `charts` plugin `theme` service. **Accessibility** - Updated `EuiModal` to set an `aria-modal` attribute and a default `dialog` role ([#7564](elastic/eui#7564)) - Updated `EuiConfirmModal` to set a default `alertdialog` role ([#7564](elastic/eui#7564)) - Fixed `EuiModal` and `EuiConfirmModal` to properly trap Safari+VoiceOver's virtual cursor ([#7564](elastic/eui#7564))

[fix] Fixed scripting issues due to improperly encoded kibana payload.

08a6f40

bevacqua assigned epixa Jun 28, 2016

bevacqua added review v5.0.0 v5.0.0-alpha4 and removed v5.0.0 labels Jun 28, 2016

spalger reviewed Jun 28, 2016
View reviewed changes

[fix] Switch to attribute-encoding

2cdb0f9

spalger reviewed Jun 28, 2016
View reviewed changes

[fix] Get rid of <meta> in favor of directive-style element.

e33fa40

bevacqua merged commit 271aa69 into elastic:master Jun 28, 2016

bevacqua unassigned epixa Jun 28, 2016

elastic-jasper mentioned this pull request Jun 28, 2016

[backport] PR #7568 to 5.0.0-alpha4 #7570

Merged

epixa added a commit that referenced this pull request Jun 28, 2016

Merge pull request #7570 from elastic/jasper/backport/7568/5.0.0-alpha4

7a7831f

[backport] PR #7568 to 5.0.0-alpha4

epixa added a commit that referenced this pull request Jun 28, 2016

Revert "Merge pull request #7568 from bevacqua/hotfix/config-xss"

dda84e9

This reverts commit 271aa69, reversing changes made to 1248b2f. This should not have been merged because tests did not pass.

epixa removed review v5.0.0-alpha4 labels Jun 28, 2016

bevacqua added a commit to bevacqua/kibana that referenced this pull request Jun 29, 2016

Revert "Revert "Merge pull request elastic#7568 from bevacqua/hotfix/…

4854f2d

…config-xss"" This reverts commit dda84e9.

elastic-jasper mentioned this pull request Jun 29, 2016

[5.0.0-alpha4] Fixed scripting issues due to improperly encoded kibana payload #7584

Merged

airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017

Merge pull request elastic#7568 from bevacqua/hotfix/config-xss

1785ccf

Fixed scripting issues due to improperly encoded kibana payload Former-commit-id: 271aa69

airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017

Revert "Revert "Merge pull request elastic#7568 from bevacqua/hotfix/…

3efbb67

…config-xss"" This reverts commit b6ff2a6 [formerly dda84e9]. Former-commit-id: 4854f2d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed scripting issues due to improperly encoded kibana payload #7568

Fixed scripting issues due to improperly encoded kibana payload #7568

bevacqua commented Jun 28, 2016 •

edited

Loading

spalger Jun 28, 2016

bevacqua Jun 28, 2016

spalger Jun 28, 2016

spalger commented Jun 28, 2016

spalger commented Jun 28, 2016

bevacqua commented Jun 28, 2016

epixa commented Jun 28, 2016

Fixed scripting issues due to improperly encoded kibana payload #7568

Fixed scripting issues due to improperly encoded kibana payload #7568

Conversation

bevacqua commented Jun 28, 2016 • edited Loading

spalger Jun 28, 2016

Choose a reason for hiding this comment

bevacqua Jun 28, 2016

Choose a reason for hiding this comment

spalger Jun 28, 2016

Choose a reason for hiding this comment

spalger commented Jun 28, 2016

spalger commented Jun 28, 2016

bevacqua commented Jun 28, 2016

epixa commented Jun 28, 2016

bevacqua commented Jun 28, 2016 •

edited

Loading