Dealing with bad html #32

kouk · 2015-10-14T06:12:20Z

Depending on the source markup there are times when the trumpet pipeline will not finish processing even though the response has been written entirely (see below). It would be great if there was a way to set a timeout and cancel the pipeline somehow. For example:

var http = require('http'),
    connect = require('connect'),
    through = require('through'),
    httpProxy = require('http-proxy');


var selects = [];
var simpleselect = {};

simpleselect.query = 'body';
simpleselect.func = function (node) {
    var ts = node.createStream();
    ts.pipe(through(null, function(){
        console.log("Injecting");
        this.queue("FOOBAR");
        this.queue(null);
    })).pipe(ts);

    ts.on('end', function() {
     console.log("BODY");
    });
}

selects.push(simpleselect);

var app = connect();

var proxy = httpProxy.createProxyServer({
    target: 'http://localhost:8002'
})

app.use(require('harmon')([], selects, true));

app.use(
  function (req, res) {
    proxy.web(req, res);
  }
);

http.createServer(app).listen(8001);

http.createServer(function(req, res) {
    res.setHeader('Content-Type', 'text/html; charset=UTF-8');
    res.setHeader('Transfer-Encoding', 'chunked');
    res.write('<html><head><title>foo</title></head><body><h1>Foo</h1>');
    /* With this line the request hangs
    res.write('<div><svg><defs><circle /></defs><circle /></svg></div>');
    */
    res.end('</body></html>');
}).listen(8002);

If you run the example and curl http://localhost:8001 it works fine. But if you uncomment the commented line and try again it hangs. I've tracked down the specific underlying reason for this in the html-select module, and I'll raise this specific issue with them. But what I'm interested in is shielding myself from these kinds of bugs (which may be triggered by broken proxied web pages) by doing setTimeout and aborting the trumpet pipeline somehow. I've tried various combinations of res.end() and tr.end() but the stream from tr.createStream just never seems to die. I would appreciate if you could show me a way, perhaps even a totally different approach.

The text was updated successfully, but these errors were encountered:

kouk · 2015-10-14T07:51:29Z

There's also a discussion here which might be related: https://github.com/substack/html-tokenize/issues/13

But all this isn't really future proof, it would be great if somehow we can call ts.end() or node.abort() and have it kill all the streams that might be blocking.

mspl13 · 2015-10-18T13:10:50Z

I'm running in the same issue using this gist. If you uncomment line 45 and comment 44 out, it won't stop loading and you can't porceed with anything.
Couldn't find any solution to this point and can't really think of any way to stop it from loading...

No9 · 2015-10-19T10:59:05Z

From the notes here I agree this feels like an upstream fix.
But thanks for the steps to reproduce.

mspl13 · 2015-10-20T13:56:56Z

Sorry for seeming impatient (I'm not, just curious) but do you already know what you gonna do about it? Reporting this as a bug to the actual module that is affected or fix it by yourself?
Since you added the 'help-wanted' tag, do you wait for somebody you know to help you out with this?

No9 · 2015-10-20T16:30:49Z

Hi @mspl13 I am not actively looking at this at the moment.
Hence the help wanted tag.
I would love somebody to help with this if they have the bandwidth.

chandransuresh · 2018-05-23T19:08:39Z

Hi @No9
I faced issue with bad HTML as well ( "end" event does not get fired for bad HTML) , I ended up adding this code to fix it:
Would you let me create a PR with a change similar to the following? (need to get the Max timeout from configuration). Thanks.

let endEmittedInterval;

let endEmittedInterval;
tr.on('finish', () => {
            let intervalCount = 0;
            endEmittedInterval = setInterval(() => {
                try {
                    intervalCount += 1;
                    // make sure read and write are completed after finish
                    if (tr._writableState.ended === true && tr._tokenize._readableState.endEmitted === true) {
                        clearInterval(endEmittedInterval);
                        _end.call(res);
                    } else if (intervalCount >= 6000) {
                        // clear the runway interval after 60 seconds
                        // TODO: change 60 seconds to server's MAXIMUM_TIMEOUT value
                        clearInterval(endEmittedInterval);
                        _end.call(res);
                    }
                } catch (err) {
                    clearInterval(endEmittedInterval);
                    _end.call(res);
                }
            }, 10);
        });

No9 · 2018-05-23T22:36:08Z

Hi @chandransuresh
Thanks for your message.

Do you know if this patch to html-select fix your issue?
https://github.com/substack/html-select/pull/23

If it does we might want to fork that along with trumpet and publishing them in there own namespace?

If not then we can consider your workaround above and putting it into the harmon constructor
something like

var opts = {};
opts.timeout = 6000;
app.use(require('../')([], selects, opts));

Let me know how you get on

chandransuresh · 2018-05-29T06:43:41Z

Hi @No9 , Thanks for the response.
I tested with the change in kouk:master branch from substack/html-select#23. No, it does not fix the issue of broken HTML. In my sample page, there are bunch of tags that does not have any matching close tags.

chandransuresh · 2018-08-07T03:32:45Z

Hi @No9 ,
Sorry for the delay in getting back. Could you give me access to create a PR for malformed HTML fix?.
The changed file is in the following clone.
https://github.com/chandransuresh/harmon-malformed-html-fix/blob/fix-for-malformed-html/index.js

Thanks,
Suresh

No9 added the help-wanted label Oct 19, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dealing with bad html #32

Dealing with bad html #32

kouk commented Oct 14, 2015

kouk commented Oct 14, 2015

mspl13 commented Oct 18, 2015

No9 commented Oct 19, 2015

mspl13 commented Oct 20, 2015

No9 commented Oct 20, 2015

chandransuresh commented May 23, 2018 •

edited

Loading

No9 commented May 23, 2018

chandransuresh commented May 29, 2018 •

edited

Loading

chandransuresh commented Aug 7, 2018

Dealing with bad html #32

Dealing with bad html #32

Comments

kouk commented Oct 14, 2015

kouk commented Oct 14, 2015

mspl13 commented Oct 18, 2015

No9 commented Oct 19, 2015

mspl13 commented Oct 20, 2015

No9 commented Oct 20, 2015

chandransuresh commented May 23, 2018 • edited Loading

No9 commented May 23, 2018

chandransuresh commented May 29, 2018 • edited Loading

chandransuresh commented Aug 7, 2018

chandransuresh commented May 23, 2018 •

edited

Loading

chandransuresh commented May 29, 2018 •

edited

Loading