Need to gsub occurrences of {{ }} in the output #1369

h-kataria · 2017-05-17T13:19:55Z

Moved gsub logic into a helper method. Having ", ', {{, }} characters in the output was causing an issue while rendering html using ngSanitize directive, needed to escape these characters to get the output to be displayed correctly on screen.

https://bugzilla.redhat.com/show_bug.cgi?id=1451352
https://bugzilla.redhat.com/show_bug.cgi?id=1444853

@syncrou @gmcculloug please test/review.

syncrou · 2017-05-17T13:24:23Z

app/views/service/_svcs_show.html.haml

 :javascript
  miq_tabs_init('#services_tab');
-  miq_bootstrap('#service_details', 'sanitizeRender');
+  miq_bootstrap('#std_output', 'miq.helpers');


@h-kataria - Just for my understanding as to what this code is doing: is miq_bootstrap applying miq.helpers to the div id?

@syncrou i am tying the div that holds standard output to miq_bootstrap object, previously we were attaching the whole outer div but in this case i only need the specific to be attached. miq.helpers includes ngSanitize directive that is being used.

h-kataria · 2017-05-17T15:28:27Z

@syncrou please test/review, also can you please upload a screenshot from the appliance for the Service details screen that was not showing the output prior to this fix.

syncrou · 2017-05-17T15:46:41Z

app/views/service/_svcs_show.html.haml

@@ -28,7 +28,7 @@
            .form-horizontal.static
              .form-group
                .col-md-9
-                  - htm = @job.raw_stdout('html').gsub('"', '\"').gsub("'", "\\\\'")
+                  - htm = @job.raw_stdout('html').gsub('"', '\"').gsub("'", "\\\\'").gsub(/{{/, '\{{').gsub(/}}/, '\}}')


@h-kataria - We'll need this .gsub(/{{/, '\{\{').gsub(/}}/, '\}\}') (and on line 41) for it to work.

gmcculloug · 2017-05-17T15:56:20Z

app/views/service/_svcs_show.html.haml

@@ -28,7 +28,7 @@
            .form-horizontal.static
              .form-group
                .col-md-9
-                  - htm = @job.raw_stdout('html').gsub('"', '\"').gsub("'", "\\\\'")
+                  - htm = @job.raw_stdout('html').gsub('"', '\"').gsub("'", "\\\\'").gsub(/{{/, '\{\{').gsub(/}}/, '\}\}')


Each gsub call is going to return a copy of the original string, since these can be large output it would make more sense to do separate gsub! calls so the substitutions happens without duplicating the string.

There might also be a way to do this in one gsub! call. For example:
'hello'.gsub(/[eo]/, 'e' => 3, 'o' => '*') #=> "h3ll*" (See https://ruby-doc.org/core-2.1.4/String.html)

syncrou

👍 Looks Good to Me

gmcculloug

@h-kataria Please add a comment to the description explaining why these characters need to be escaped. Otherwise, I am good with the change.

h-kataria · 2017-05-17T19:06:46Z

@gmcculloug commit comment has been updated.

syncrou

I found an issue with sanitize_output in its current form

syncrou · 2017-05-17T19:22:38Z

app/controllers/service_controller.rb

@@ -141,6 +141,15 @@ def service_form_fields

  private

+  def sanitize_output(stdout)


htm = stdout.gsub('"', '\"') htm.gsub!("'", "\\\\'") htm.gsub!(/{{/, '\{\{') htm.gsub!(/}}/, '\}\}') htm

This is actually a copy of the gsub pattern that will work.

The files coming back from Ansible are prefixing {{ with a " which needs to be escaped first before we can gsub! on the returned string. Angular is blowing up on the {{ which also need to be escaped. Not sure this solution is the most efficient - but it is the exact code running on the appliance that allows the output to show.

The substitution needs to change: u\"{{ manageiq_mach | default('localhost') }}\" into \{\{ manageiq_mach | default(\'localhost\') \}\}

h-kataria · 2017-05-17T20:05:31Z

@syncrou updated sanitize_output method

Moved gsub logic into a helper method. Having ", ', {{, }} characters in the output was causing an issue while rendering html using ngSanitize directive, needed to escape these characters to get the output to be displayed correctly on screen. Have to gsub " on the first line and then gsub for rest of the characters in a one liner works fine. https://bugzilla.redhat.com/show_bug.cgi?id=1451352 https://bugzilla.redhat.com/show_bug.cgi?id=1444853

miq-bot · 2017-05-17T20:19:22Z

Checked commit h-kataria@c276603 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🍰

syncrou · 2017-05-17T20:39:38Z

👍 👍 (re re) Approve

Need to gsub occurrences of {{ }} in the output (cherry picked from commit f8fab5e) https://bugzilla.redhat.com/show_bug.cgi?id=1451920 https://bugzilla.redhat.com/show_bug.cgi?id=1446245

simaishi · 2017-05-17T21:42:11Z

Fine backport details:

$ git log -1
commit 1048ffdbd63fdba91023f42efbad4804728ae254
Author: Dan Clarizio <[email protected]>
Date:   Wed May 17 13:51:26 2017 -0700

    Merge pull request #1369 from h-kataria/stdout_fix
    
    Need to gsub occurrences of {{ }} in the output
    (cherry picked from commit f8fab5ea90ab51071663d895e683ef76f0f578f0)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1451920
    https://bugzilla.redhat.com/show_bug.cgi?id=1446245

h-kataria added blocker bug fine/yes services labels May 17, 2017

h-kataria assigned dclarizio May 17, 2017

syncrou reviewed May 17, 2017

View reviewed changes

h-kataria force-pushed the stdout_fix branch from f17717a to 498baaf Compare May 17, 2017 15:26

h-kataria changed the title ~~Reverting changes to show html standard output as nice colorful output~~ Need to gsub occurrences of {{ }} in the output May 17, 2017

syncrou suggested changes May 17, 2017

View reviewed changes

h-kataria force-pushed the stdout_fix branch from 498baaf to 6d5e0d0 Compare May 17, 2017 15:48

gmcculloug reviewed May 17, 2017

View reviewed changes

h-kataria force-pushed the stdout_fix branch from 6d5e0d0 to d29a307 Compare May 17, 2017 18:09

syncrou approved these changes May 17, 2017

View reviewed changes

gmcculloug approved these changes May 17, 2017

View reviewed changes

h-kataria force-pushed the stdout_fix branch 2 times, most recently from 050e034 to 1dad2ee Compare May 17, 2017 19:06

syncrou suggested changes May 17, 2017

View reviewed changes

syncrou approved these changes May 17, 2017

View reviewed changes

h-kataria force-pushed the stdout_fix branch from 1dad2ee to caf64a1 Compare May 17, 2017 20:04

h-kataria force-pushed the stdout_fix branch from caf64a1 to c276603 Compare May 17, 2017 20:13

dclarizio merged commit f8fab5e into ManageIQ:master May 17, 2017

dclarizio added this to the Sprint 61 Ending May 22, 2017 milestone May 17, 2017

simaishi added fine/backported and removed fine/yes labels May 17, 2017

h-kataria deleted the stdout_fix branch June 27, 2017 20:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Need to gsub occurrences of {{ }} in the output #1369

Need to gsub occurrences of {{ }} in the output #1369

h-kataria commented May 17, 2017 •

edited

Loading

syncrou May 17, 2017

h-kataria May 17, 2017

h-kataria commented May 17, 2017

syncrou May 17, 2017

gmcculloug May 17, 2017

syncrou left a comment

gmcculloug left a comment

h-kataria commented May 17, 2017

syncrou left a comment

syncrou May 17, 2017 •

edited

Loading

syncrou May 17, 2017

h-kataria commented May 17, 2017

miq-bot commented May 17, 2017

syncrou commented May 17, 2017

simaishi commented May 17, 2017

		@@ -141,6 +141,15 @@ def service_form_fields

		private

		def sanitize_output(stdout)

Need to gsub occurrences of {{ }} in the output #1369

Need to gsub occurrences of {{ }} in the output #1369

Conversation

h-kataria commented May 17, 2017 • edited Loading

syncrou May 17, 2017

Choose a reason for hiding this comment

h-kataria May 17, 2017

Choose a reason for hiding this comment

h-kataria commented May 17, 2017

syncrou May 17, 2017

Choose a reason for hiding this comment

gmcculloug May 17, 2017

Choose a reason for hiding this comment

syncrou left a comment

Choose a reason for hiding this comment

gmcculloug left a comment

Choose a reason for hiding this comment

h-kataria commented May 17, 2017

syncrou left a comment

Choose a reason for hiding this comment

syncrou May 17, 2017 • edited Loading

Choose a reason for hiding this comment

syncrou May 17, 2017

Choose a reason for hiding this comment

h-kataria commented May 17, 2017

miq-bot commented May 17, 2017

syncrou commented May 17, 2017

simaishi commented May 17, 2017

h-kataria commented May 17, 2017 •

edited

Loading

syncrou May 17, 2017 •

edited

Loading