Make template_format checking use a whitelist

duncanbeevers / rails_view_annotator

Wraps the rendering of Rails partials with html comments indicating the disk location of the rendered partial

32 stars 9 forks source link

Make template_format checking use a whitelist #16

Open gerrywastaken opened 7 years ago

gerrywastaken commented 7 years ago

Code such as the follow contains :html in the template_formats list, but is made invalid if a html comment is appended to it.

render(
  partial: 'scss partial',
  locals: { main_color: '#f0f' },
  formats: :scss
)

I've changed the template_format checking code so that it only applies the template when we are absoutely certain that the template is in the expected format.

gerrywastaken commented 7 years ago

Do you see any issues with this change?

duncanbeevers commented 7 years ago

@gerrywastaken I haven't touched this gem in several years.

The change seems reasonable but I don't have any day-to-day interaction with the gem by which to measure the value of the change.

gerrywastaken commented 7 years ago

I'm not sure where to go from here, but what you have said is very understandable. Thanks for looking over the change anyway.

duncanbeevers commented 7 years ago

@gerrywastaken I see three paths forward

Copy the gem code into your app's codebase with the changes you want; no more gem dependency
Fork the gem and publish your own differently-named version and use that.
I can hand over the keys to the gem if you'd like to simply take over. Then you could publish your own "official" version and use that in your app.

gerrywastaken commented 7 years ago

I'm thinking either option 2 or 3. What would you prefer?

duncanbeevers commented 7 years ago

@gerrywastaken My apologies; this fell off my radar. If you would still like an ownership role on the rails_view_annotator gem, let me know which email address you'd like given ownership, and I'll take care of it.

gerrywastaken commented 7 years ago

@duncanbeevers No worries at all. You can send it to

['rubygems', 'caulfield.me'].join('@')

;-)

duncanbeevers commented 7 years ago

@gerrywastaken In order to become a gem owner, you must have a rubygems.org account registered with the given email address.

suan commented 6 years ago

@gerrywastaken Could u give me ownership so I can merge this fix? My email is ['yeosuanaik', 'gmail.com'].join('@')

Actually, would you mind also giving commit access to this repo? That should make things easier too

gerrywastaken commented 6 years ago

@suan Oh I'm not a maintainer yet.

@duncanbeevers Super sorry I missed your first comment. Suan's comment alerted me to it.

It looks like I got the email slightly wrong. It's actually: ['rubygems.org', 'caulfield.me'].join('@')

Or you could add @suan instead.