bublejs / buble

https://buble.surge.sh
MIT License
869 stars 67 forks source link

Parsing code with carelessly crafted comments hangs node at 100% CPU #202

Closed strongholdmedia closed 5 years ago

strongholdmedia commented 5 years ago

I have this test case. (It was part of a way bigger block, I just managed to narrow it down to this).

As it is, it removes the comma at the end of line 15. It also removes it if it is in the middle of the line, I tested it. (And as such, derails buble-loader when used with webpack, as this is surely a syntax error.)

If I remove the line comments at the bottom - it always works. If I swap the two imports at top - it works. Strange.

If I remove the block level comment COMPLETELY (with the code inside), it hangs with node using 100% CPU and not doing anything in 8 hours.

This seems to me an optimistic regular expression greediness issue. But I am unsure due to the hang (that was the original issue I've found). That's why I am less eager to offer a more specific title for this issue.

Test case:

const JSShim = require("./lib/helper/js-shim");
const elm = require("preact-compat").createElement;

function Test(props) { }
Test.prototype.render = function render()
{
    return elm("div", { className: "col02" },
        this.state.enabled && !this.state.readOnly
            ? elm(
                "a",
                {
                    className: "vs button",
                    title: this.props.application.labels.controller.buttonCaptionWhatever /*,
                    onClick: JSShim.bind(function(event) { return this.props.removeClick(this.props.index, event); }, this) */
                },
                elm("span", { className: "fas fa-2x fa-times text-danger" })
            )
            : null
// elm(
// )

    );
};

module.exports = Test;
strongholdmedia commented 5 years ago

I am fairly sure this is related to #175, but it is not a duplicate, as it is said to be resolved and fixed.

mourner commented 5 years ago

@strongholdmedia can you try this out with the master version? #175 has been fixed but we didn't have a new release on NPM yet.

strongholdmedia commented 5 years ago

Thanks for the heads up.

I can indeed confirm that in the master version, all issues seem rectified.

In fact I was dumb, since I thought that the "3 months ago" NPM publish date meant after this version.

So please make a release, if possible, as I am absolutely not keen on having github and its apparently stochastic rate limits in our deployment pipeline.

strongholdmedia commented 5 years ago

Duplicate of #175