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

Method to Disable Rendering with Safebuffer #1238

Closed
basex opened this issue Apr 10, 2013 · 14 comments
Closed

Method to Disable Rendering with Safebuffer #1238

basex opened this issue Apr 10, 2013 · 14 comments
Labels
enhancement feature
Milestone
0.11.3

Comments

@basex
Copy link
Contributor

basex commented Apr 10, 2013

Shouldn't there be a method to disable Safebuffer for your app the same way you can disable the CSRF protection?

set :allow_disabled_csrf, true

My problem is that I'm using fast_gettext a lot on templates, so I have many strings like

_("a <strong>string</strong> to translate.")

and

_("string to <a href='%{url}'>translate</a>.") % {:url => '/link'}

Adding <%== or .html_safe everywhere doesn't seem a good idea. This are just static pages without forms, so I think it should be possible to neglect the Safebuffer protection.

For reference, this is related with #1031
IRC chat about this topic: http://irclogger.com/.padrino/2013-04-09#1365558257
@shipstar
Copy link

+1

I'm rendering a raw HTML template to make an index.html file inside a Dropbox folder. I'm currently hacking around it with:

def default_html
  Tilt.prefer Tilt::ErubisTemplate, :erb
  Tilt.new('ui/views/templates/index.html.erb').render(self)
  Tilt.prefer Padrino::Erubis::Template, :erb
end

but I'd much rather be able to pass escape_html: false like I can with the default Erubis::Eruby engine.

I'll try to throw a pull request together if it seems reasonable.

UPDATE: I think escape_html and a global flag per @basex could be handled generally the same way within the engine.

@dariocravero dariocravero mentioned this issue Apr 10, 2013
Closed
@dariocravero
Copy link

@skade what are your thoughts on this? I think that allowing users to disable the use of SafeBuffer makes sense. /cc @padrino/core-members

@nesquena
Copy link
Member

I see no reason not to allow it to be disabled if the user wants...
—
Nathan Esquenazi
CodePath Co-founder
http://thecodepath.com

On Wed, Apr 10, 2013 at 3:47 PM, Darío Javier Cravero
notifications@github.com wrote:

@skade what are your thoughts on this? I think that allowing users to disable the use of SafeBuffer makes sense. /cc @padrino/core-members

Reply to this email directly or view it on GitHub:
#1238 (comment)

@dariocravero
Copy link

I was thinking about the implementation for this... Would it be a good idea to provide a Padrino::SafeBuffer implementation that is just a blank implementation of the assumptions that AS's SafeBuffer does html_safe, html_safe?, etc. that default to returning whatever the current string is. Then, if the app has a setting like :sanitise_html, true is enabled it uses AR otherwise it doesn't? Of course, this would be enabled by default. Thoughts?

@skade
Copy link
Contributor

skade commented Apr 11, 2013

I am not really convinced. Wouldn't it be better to just make the helpers in question just generate html_safe strings? I don't see that many issues in that regard when talking about gettext.

@dariocravero
Copy link

Yeah @skade, something along those lines was my initial proposition to @basex too but some people may still want to fully disable this feature instead of wrapping it around with helpers. Of course they need to understand the security issues associated with it, but that's up to them too.

@shipstar
Copy link

@skade My specific issue is that an entire template is getting escaped. In my example above, 'ui/views/templates/index.html.erb' is (excerpt):

<html>
    <body>
        Welcome to <%= name %>!
    </body>
</html>

That's generating "<html>" (and so on.)

I could convert that to a String and html_safe it, but it seems indirect. I'd rather just have the option to pass escape_html: false to the engine.

@stephenreid321
Copy link

This is preventing me from upgrading to 0.11. Please can we have an option to disable the safebuffer asap?

@DAddYE
Copy link
Member

DAddYE commented Apr 13, 2013

I agree, also for performance purposes

Sent from my iPhone

On 13/apr/2013, at 09:29, Stephen Reid notifications@github.com wrote:

This is preventing me from upgrading to 0.11. Please can we have an option
to disable the safebuffer asap?

—
Reply to this email directly or view it on
GitHubhttps://github.com//issues/1238#issuecomment-16336089
.

@skade
Copy link
Contributor

skade commented Apr 13, 2013

What performance purposes? A SafeBuffer is a String and concatenating two SafeBuffers or a String and SafeBuffer is as cheap as concatenating to normal strings, including all optimizations the interpreter does.

@skade
Copy link
Contributor

skade commented Apr 13, 2013

Probably the cleanest way to do this is to declare Strings as html_safe if the feature should be disabled. I am not a fan of this options, still, though. It points to an application problem that should be fixed. There is a reason why you cannot disable this behaviour in most engines/frameworks that use similar techniques.

class String
  def html_safe?
    true
  end
end

@tiagomatos
Copy link

+1 This is crucial for us.

@skade
Copy link
Contributor

skade commented Apr 16, 2013

@tiagomatos Just to gather knowledge: could you please go into detail about your problems?

@karlfreeman karlfreeman mentioned this issue Jun 7, 2013
Merged
@DAddYE
Copy link
Member

DAddYE commented Jul 1, 2013 

# lib/disable_safe_buffer.rb
String.send(:define_method, :html_safe?){ true }

We should document this and that's it.

@DAddYE DAddYE closed this as completed Jul 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement feature
Projects
None yet
Milestone
0.11.3
Development

No branches or pull requests
8 participants
@nesquena @DAddYE @stephenreid321 @skade @shipstar @basex @dariocravero @tiagomatos