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

What about escaping specific BEMJSON field, e.g. 'contentSafe'? #179

Open
f0rmat1k opened this issue Feb 15, 2017 · 14 comments
Open

What about escaping specific BEMJSON field, e.g. 'contentSafe'? #179

f0rmat1k opened this issue Feb 15, 2017 · 14 comments

Comments

@f0rmat1k
Copy link
Contributor

Something like this: { block: 'button', contentSafe: '<script>alert('you shall not pass!')</script> }

@miripiruni
Copy link

@f0rmat1k escaping or contentSafe? It’s opposite things. Escaping any fields means you don’t trust it’s value. But naming the field contentSafe we assume that its content is safe and it’s value should be output as is.

In BH you can use html field. See test: https://github.com/bem/bh/blob/master/test/test.html.js#L14

@qfox
Copy link
Member

qfox commented Feb 15, 2017

Feels like it's the same as safe/unsafe to mark fields as "need escaping" and "does not need escaping" flags.

I like the way we do it in bem-xjst: { content: { html: '<script>alert(1);</script>' } } (same as safe: '<script>alert(1);</script>', but I'm agree that opposite will be useful too to not call escaping method directly.

@f0rmat1k
Copy link
Contributor Author

f0rmat1k commented Feb 15, 2017

@miripiruni I just wanted to say, that now i have to escape handy any content

{
   content: escape('content')
}

Global option isn't useful.
Maybe BH should really force escape content field? And do like react:

{
   dangerouslyContent: '<span>content</span>'
}

Anyway for me would be enough some field where i can insert content for escaping.

@Yeti-or
Copy link
Member

Yeti-or commented Mar 17, 2017

@f0rmat1k I don't understand why option escapeContent:true doen't do what you want?

@f0rmat1k
Copy link
Contributor Author

@Yeti-or because it affects whole project

@qfox
Copy link
Member

qfox commented Mar 17, 2017

Like that?

bh.match('*', function(ctx, json){
  if(json.dangerousContent) {
    json.content = bh.escape(json.dangerousContent)
  }
})

@qfox
Copy link
Member

qfox commented Mar 17, 2017

Bemjson = {
  content: [
    'this wont be escaped',
    { unsafe: true, content: [
      'But this will be escaped'
    ] }
  ]
}

And...

bh.match('*', function...
  if(json.unsafe) {
    ctx.unsafe = true
    res = ctx.applyBase()
    ctx.unsafe = false
    return res
  }

bh.match('*', function...
  if(ctx.unsafe) escape content manually

@Yeti-or
Copy link
Member

Yeti-or commented Mar 17, 2017

react escaping affects whole project too

@f0rmat1k
Copy link
Contributor Author

@Yeti-or but react has dangerouslySetInnerHtml

@qfox
Copy link
Member

qfox commented Mar 18, 2017

@f0rmat1k BH has content: { tag: false, html: ... } that works like dangerouslySetInnerHtml: ...

@f0rmat1k
Copy link
Contributor Author

@zxqfox So i will have to create .bh for every block (usually priv is enought for me). It isn't as simple as specific bemjson field. But thank you for advice.

@mishanga
Copy link
Member

Можно реализовать компрометирующую логику через tParam: выставлять его для всех детей где-то на уровне блока, у которого есть шаблон. То есть компрометировать ветку в BEMJSON, а не узел.

@qfox
Copy link
Member

qfox commented Mar 20, 2017

Оу, когда я писал про стх, я имел ввиду tParam для поддерева. Я поправлю сниппет

@qfox
Copy link
Member

qfox commented Mar 20, 2017

Без tParam: https://goo.gl/JvaD4D Кажется, что так даже лучше.
С tParam: https://goo.gl/sAFRbg https://goo.gl/RRU1nW

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

5 participants