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

Timeout ignored for json request and streaming body #3341

Closed
mochja opened this issue Sep 15, 2020 · 3 comments
Closed

Timeout ignored for json request and streaming body #3341

mochja opened this issue Sep 15, 2020 · 3 comments
Labels

Comments

@mochja
Copy link

mochja commented Sep 15, 2020

Summary

Expecting a request to timeout when JSON response could not be delivered before timeout, although timeout does not take place for streaming responses, with socket activity before timeout.

Simplest Example to Reproduce

var http = require('http');
var r = require('request');

const server = http.createServer(function(req, res) {
	res.writeHead(200, { 'Content-Type': 'text/plain' });
	res.write('"a')

	const delay = (end, cb) => setTimeout(() => {
		res.write('e')
		if (end) {
			res.end('"')
		}
		cb()
	}, 800)

	let n = 999999
	delay(false, function next() {
		n -= 1
		const end = n < 1
		delay(end, end ? () => (void 0) : next)
	})
});

server.listen('3000')

r({
	url: 'http://localhost:3000',
	json: true,
	timeout: 1000
}, (err, res) => {
	console.log(err, res)

	server.close()
})

Expected Behavior

According to documentation, timeout option does not handle this case, but combined with json option set to true, I would expect timeout to apply here as well.

Current Behavior

Such request can ran theoretically forever.

Possible Solution

  • Apply overall request timeout when json property is set to true.
  • Introduce another timeout variable to handle response timeout. Including body.

Context

I was just wondering if this would work as I was reading the documentation and thought this might be security issue.

Your Environment

software version
request 2.88.2
node 12.13.0
npm 6.13.4
Operating System Mac OS 10.15.6 (19G2021)
@Verhov
Copy link

Verhov commented Dec 20, 2020

Have a similar issue bda-research/node-crawler#371. Have you found any workaround?

@mike442144
Copy link

mike442144 commented Dec 25, 2020

Hey, I read your code which keeps writing bytes to socket, in this case the socket connection is active and normally we cannot close or destroy an active connection that behavior is the same as working one. We close inactive one. Also this is not JSON-related issue, whatever document type you want what you do is downloading, and how can we close a downloading procedure. Finally, such request won't ran forever, you can always finish downloading some day unless the server would not like you to. And so, the content of the server is not what you want, add it to blacklist instead.
Feel free to discuss if any more question. hope it helps.

@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 9, 2022
@stale stale bot closed this as completed Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants