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.

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

Javascript escaping invalid, generates SyntaxError: expected expression, got '&' #940

Closed
javinto opened this issue Jul 6, 2017 · 29 comments

Comments

@javinto
Copy link

javinto commented Jul 6, 2017

The 5.0.0 release notes refers to:

HTML escape interpolated code in filters. #770 (Matt Wildig)

:javascript
    #{JSON.generate(foo: "bar")}
  Haml 4 output: {"foo":"bar"}
  Haml 5 output: {"foo":"bar"}

However, I have (Haml 5.0.1):

:javascript
  var list1= ["a", 'b', 'c'];
  var hash1= {a: "a", b: 'b'};
  var list2= #{JSON.generate( [:a, :b, :c])};
  var hash2= #{JSON.generate(foo: "bar")};

In the browser this results into:

<script>
  var list1= ["a", 'b', 'c'];
  var hash1= {a: "a", b: 'b'};
  var list2= [&quot;a&quot;,&quot;b&quot;,&quot;c&quot;];
  var hash2= {&quot;foo&quot;:&quot;bar&quot;};
</script>

The browsers runs into an error on line 4 (list2) and 5 (hash2):

SyntaxError: expected expression, got '&'

@k0kubun
Copy link
Member

k0kubun commented Jul 10, 2017

This is intentional, not a bug. You should fix template like this (using data attribute) railsadminteam/rails_admin#2870 or disable HTML escape globally.

Haml::Template.options[:escape_html] = false

@k0kubun k0kubun closed this as completed Jul 10, 2017
@javinto
Copy link
Author

javinto commented Jul 17, 2017 via email

k0kubun added a commit that referenced this issue Jul 17, 2017
@k0kubun
Copy link
Member

k0kubun commented Jul 17, 2017

Does this mean that the documentation is outdated?

Yes, definitely. I fixed it 965e66e. Thank you for reporting it.

@marcelkooi
Copy link

@k0kubun is it possible to set the escape_html option for a particular filter? For instance,

:javascript{ escape_html: false }

Just looking to keep the same behavior as before.

@k0kubun
Copy link
Member

k0kubun commented Jul 26, 2017

is it possible to set the escape_html option for a particular filter? For instance,
:javascript{ escape_html: false }

No, as said in #770.

Just looking to keep the same behavior as before.

In Rails, you can call html_safe to value. Otherwise no idea.

@midnyt-simlyn
Copy link

I still don't understand how this works, or why it was introduced. What could previously be done with:

:javascript
  var array = #{@user.all.to_json}

...now results in a broken JSON output. Even if we ignore the why, I still don't know how to use :escape_html? Should it be used like in the above comment, i.e:

:javascript{ escape_html: false }
  var array = #{@user.all.to_json}

Or does :escape_html go somewhere else?

@k0kubun
Copy link
Member

k0kubun commented Jul 31, 2017

It seems that you use ActiveRecord and it's likely that you can use html_safe or raw as I said. Why don't you use that? Did you check #770?

:javascript
  var array = #{@user.all.to_json.html_safe}

Even if we ignore the why, I still don't know how to use :escape_html?

Haml::Template.options[:escape_html] = false

This would also solve your problem if you're using Rails.

I still don't understand how this works, or why it was introduced.

That's because there are possibly vulnerable cases in some string interpolation. Following code can generate unexpected code in Haml 4, but not in Haml 5 with escape_html: true.

- foo = "null;</script> ANY HTML HERE <script>"
:javascript
  var array = #{foo}

Does it make sense?

@javinto
Copy link
Author

javinto commented Jul 31, 2017 via email

@midnyt-simlyn
Copy link

Ah nice, yeah that makes sense. Although html_safe is not an option for me, it fails to render correctly because of how I filter the output with includes:

:javascript
  var array = #{@available_shifts.to_json(:include => { :ward => {:include => :location} })}

If I add html_safe to the above output, like so:

:javascript
  var array = #{@available_shifts.to_json.html_safe(:include => { :ward => {:include => :location} })}

...I get a Uncaught reference error in console.

I'll use the Haml::Template.options[:escape_html] = false, seems the best option.

@k0kubun
Copy link
Member

k0kubun commented Jul 31, 2017

Please try one of followings if your application is on production:

:javascript
  var array = #{@available_shifts.to_json(:include => { :ward => {:include => :location} }).html_safe}
:javascript
  var array = #{raw @available_shifts.to_json(:include => { :ward => {:include => :location} })}

@midnyt-simlyn
Copy link

Tried both of those and no difference. Adding html_safe to after the include makes no difference to the JSON (still rendered with "&quote;"), and adding raw before the render also makes no difference. Both result in the same as not having them at all.

@k0kubun
Copy link
Member

k0kubun commented Jul 31, 2017

Hmm, it sounds like another issue. Let me check.

@midnyt-simlyn
Copy link

Also worth noting that I can't actually just put

var array = #{@available_shifts.to_json(:include => { :ward => {:include => :location} })}

I have to wrap the Rails section in quotes for it to do anything, i.e

var array = "#{@available_shifts.to_json(:include => { :ward => {:include => :location} })}"

@k0kubun
Copy link
Member

k0kubun commented Jul 31, 2017

var array = "#{@available_shifts.to_json(:include => { :ward => {:include => :location} })}"

You need string in array, not raw array? It would be broken if the json includes double quotes. I would write it you need JSON in string:

:javascript
  var array = #{@available_shifts.to_json(:include => { :ward => {:include => :location} }).dump.html_safe};

Anyway, I tried Haml 5.0.1 and html_safe on Rails, and it just worked. Probably you do or have something wrong.

@midnyt-simlyn
Copy link

Ok thanks, I'll take another look.

@Justin-Maxwell
Copy link

Justin-Maxwell commented Aug 29, 2017

New to Haml...

TL;DR - in 'production' haml/template.rb forces :escape_html = true, so setting it false in the initializer doesn't work!

After hours of trying to work out how to get a Ruby hash into a JavaScript object within a :javascript block (because should be easy, right?), and then, with reluctance, setting the global :escape_html option (because, outside of a few places, like :javascript, encoding to entities makes sense, right?)

I finally had hashes in objects - or so I thought...

Deploy to the server, and all the &quot; I has been fighting suddenly came back to haunt me. Gone in development, but not in production. For some reason even ...
Haml::Template.options[:escape_html] = false
... wasn't working. I was not a happy hamler!

So anyway, a long while later of running production locally with breakpoints to narrow down what was going on - it turns out that, during initialization, in production, there's a line towards the end of
/haml-5.0.2/lib/haml/template.rb that gets processed right after initializers/haml.rb that says:
Haml::Template.options[:escape_html] = true

:-/

But, fortunately, in this thread, (checking closed issues to see if it had been reported previously) I have learn't of 'html_safe' - something not mentioned as far as I can tell anywhere in the docs? [I know it's not a Haml feature]

And now I know that, in a :javascript block var = #{hash.to_json.html_safe} works fine. Still seems a bit verbose to have to put that in everywhere. I'd think a Rails-oriented HTML page generator would deal with Hash→JS-Object without requiring much prior-knowledge or thought. But if that's how it's to be...

...please add a mention of hash.to_json.html_safe in a couple of places, e.g.

This example in the Ruby Interpolation: #{} section of the reference:

:javascript
  $(document).ready(function() {
    alert(#{@message.to_json});
  });

... perhaps the FAQ, and also mention html_safe in the [Escaping HTML] section of the docs, to make it easier for the noobs who come after me... :-)

@baelter
Copy link

baelter commented Sep 15, 2017

What about non rails users that don't have raw or html_safe??

@k0kubun
Copy link
Member

k0kubun commented Sep 15, 2017

Use data attribute. I've never experienced the case that data attribute couldn't be used for just injecting JSON from Ruby world.

@baelter
Copy link

baelter commented Sep 15, 2017

example please

@k0kubun
Copy link
Member

k0kubun commented Sep 15, 2017

@baelter
Copy link

baelter commented Sep 15, 2017

What if i want it as a js variable?

@k0kubun
Copy link
Member

k0kubun commented Sep 15, 2017

var variable = document.getElementById("foo").getAttribute("data-bar");
// or possibly
var variable = JSON.parse(document.getElementById("foo").getAttribute("data-bar"));

@baelter
Copy link

baelter commented Sep 15, 2017

So the suggested solution is something like:

#myData{style: "display: none", 'data-mydata': myData.to_json}
:javascript
  var myData = JSON.parse(document.querySelector('#myData').dataset.myData);

jiiikes

@k0kubun
Copy link
Member

k0kubun commented Sep 15, 2017

Yes. Sorry for taking your time to migrate code in that way.

@jeremywadsack
Copy link

I second @Justin-Maxwell's suggestion of documentation updates. This hit us with an update to haml-rails which happened to pull in haml 5.x and I didn't even notice that there would be a change. In testing we never saw this until we pushed to production. (Thank you, @k0kubun for maintaining a CHANGELOG, which haml-rails doesn't have.)

For Rails apps and JSON, using raw (...) or adding .html_safe is just marking the content as "safe". That assumes you've already cleared it of tainted data. This matches the old behavior but you can do better with #json_escape:

:javascript
  $(document).ready(function() {
    alert(#{raw json_escape(@message.to_json)});
  });

@wjordan
Copy link
Contributor

wjordan commented Feb 20, 2018

Following up on #940 (comment):

TL;DR - in 'production' haml/template.rb forces :escape_html = true, so setting it false in the initializer doesn't work!

I managed to reproduce this consistently, and it actually seems like a subtle change in behavior introduced in Haml 5 that did not behave the same in Haml 4. I'm not sure this change in behavior is intentional, and this subtle difference can be quite dangerous considering its production-environment-specific behavior. I will follow-up on this in a linked issue.

@wjordan
Copy link
Contributor

wjordan commented Mar 1, 2018

For others still facing migration pains related to this issue, I've opened a PR #984 that adds an :escape_interpolated_html option for backwards compatibility with v4. Using the PR, set Haml::Template.options[:escape_interpolated_html] = false in an initializer to get the old version 4 behavior without needing to refactor the rest of your application code.

lostie added a commit to ministryofjustice/Claim-for-Crown-Court-Defence that referenced this issue Apr 25, 2018
@eric-norcross
Copy link

+1

@shepmaster
Copy link

I wrote a little script to help us find cases of interpolation inside of HAML :javascript filters:

require 'haml'

# Couldn't get `Haml::Util.handle_interpolation` to work *shrug*
def scan_for_escape(t)
    scanner = StringScanner.new(t)
    yield scanner while scanner.scan_until(/#\{/)
end

def check(node, fname)
  if node.type == :filter && node.value[:name] == "javascript"
    interpolations = []

    scan_for_escape(node.value[:text]) do |scanner|
      start_pos = scanner.pos
      Haml::Util.balance(scanner, '{', '}', 1)
      end_pos = scanner.pos
      matching = scanner.string[start_pos...end_pos]
      interpolations << matching
    end

    not_safe = interpolations.reject { |i| i.match /.to_json.html_safe\s*}\z/ }

    unless not_safe.empty?
      puts "#{fname}:#{node.line}:"
      not_safe.each do |b|
        puts "  #{b}"
      end
    end
  else
    node.children.each do |n|
      check(n, fname)
    end
  end
end

ARGV.each do |fname|
  file_contents = File.read(fname)
  node = Haml::Parser.new({}).call(file_contents)
  check(node, fname)
end

run as something like bundle exec ruby haml_check.rb $(find . -name '*haml')

In our case, we wanted to make sure every interpolation ended with to_json.html_safe; you may need to tweak the script for your own case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants