codsen / codsen

a monorepo of npm packages
MIT License
191 stars 27 forks source link

string-strip-html: removes <%= %> tags #2

Closed cossssmin closed 3 years ago

cossssmin commented 3 years ago

Package's name

string-strip-html@8.0.1

Describe the bug

string-strip-html is removing <%= %> tags along with everything inside them.

To Reproduce

const {stripHtml} = require('string-strip-html')

const plaintext = stripHtml('<div>My variable: <%= @var %></div>').result

This outputs: My variable:

Expected behavior

Should output: My variable: <%= @var %>

Additional context

Reported in https://github.com/maizzle/framework/issues/394 where we currently use v7.0.2, but I've just tested with v8.0.1 and the bug is still present.

P.S.: it's great to have you back here 🥳

revelt commented 3 years ago

hi Cossmin! Long time no see! I'll get it fixed. Sorry about the bug.

revelt commented 3 years ago

OK, so it seems I've coded a safeguard against <%@ which was too precise. What I've just did is I've weakened the clause, to skip if <% is detected. That should cover <%= or whatever variation of <%...

revelt commented 3 years ago

fixed in just-released 8.1.0 — proof: https://runkit.com/embed/ehqoy0nnhvud By the way, there's also stristri, the alternative parsing-based stripping tool (runs from codsen-tokenizer); GUI on https://stristri.com still work-in-progress

cossssmin commented 3 years ago

Thanks Roy, works as expected now 👍

cossssmin commented 3 years ago

Hi Roy,

@beerlington reported an interesting edge case.

This version works:

<a href="https://example.com/test?param1=<%= @param1 %>">click me</a>

which correctly produces:

click me
https://example.com/test?param1=<%= @param1 %>

However, when there are multiple params in the URL, it produces something funky:

<a href="https://example.com/test?param1=<%= @param1 %>&param2=<%= @param2 %>">click me</a>

produces:

¶m2=<%= @param2 %>">click me
revelt commented 3 years ago

on it. sorry about that

revelt commented 3 years ago

It's already fixed on local, I'm aiming to release this weekend. Algorithm wise, I DRY'ed up all asserts on "<" (assumption that tag should start here) or ">" (assumption that tag would end here) into separate functions, then beefed them up at that single location with checks, do % precede/follow. Originally, I had added the check against % following a single opening, on only one clause, inline. Now it's checking everywhere, properly. It's a good future-proof measure, we'll be able to supplement more templating language types that way. Maybe another templating language like <$... $> or something...

revelt commented 3 years ago

CI is still releasing, ticket closed automatically, it will be out in around an hour (fingers crossed)

revelt commented 3 years ago

8.2.0 has been released, please check

cossssmin commented 3 years ago

Just tested, works well - thanks Roy!