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

Method to indicate thar request has body #1095

Open
zdm opened this issue Aug 21, 2024 · 16 comments
Open

Method to indicate thar request has body #1095

zdm opened this issue Aug 21, 2024 · 16 comments

Comments

@zdm
Copy link

zdm commented Aug 21, 2024

Is it possible to add method hasBody(), which will return true if request has body, in order to understand that we need to read it using onData.

@uasan
Copy link

uasan commented Aug 21, 2024

Can check request.getHeader('content-length')

@zdm
Copy link
Author

zdm commented Aug 21, 2024

Data can be chunked, without known content length.

@uNetworkingAB
Copy link
Contributor

hasBody is a good addition

@e3dio
Copy link
Contributor

e3dio commented Aug 21, 2024

const hasBody = req => Number(req.getHeader('content-length')) || req.getHeader('transfer-encoding').includes('chunked');

@zdm
Copy link
Author

zdm commented Aug 21, 2024

yes, this will work
please, close, this issue, if you don;t want to add such method to the uws.

@uasan
Copy link

uasan commented Aug 21, 2024

The client can send 0 bytes

!!+req.getHeader('content-length')

@e3dio
Copy link
Contributor

e3dio commented Aug 21, 2024

@uasan yes '0' string needs to be properly accounted for, I fixed the function

@webcarrot
Copy link

// Several values can be listed, separated by a comma
Transfer-Encoding: gzip, chunked

req.getHeader('transfer-encoding')?.includes('chunked')

@zdm
Copy link
Author

zdm commented Aug 21, 2024

req.getHeader('transfer-encoding')?.toLowerCase().includes('chunked')

@uNetworkingAB
Copy link
Contributor

Obviously this should be hasBody() as we already do standards compliant check for this internally. So it's just setting a boolean and returning it via hasBody()

@e3dio
Copy link
Contributor

e3dio commented Aug 21, 2024

Usually you want to differentiate between standard request uploads with a 'content-length' and potentially unlimited stream of data with 'transfer-encoding: chunked', a simple hasBody would not help with that, encourages bad practice I think, maybe some people need it for something ?

const contentLength = Number(req.getHeader('content-length'));

const transferEncoding = req.getHeader('transfer-encoding');

if (contentLength) {

   const body = await getBody(res);

} else if (transferEncoding.includes('chunked')) {

   await processStream(res);

} else {

   console.log('no body');

}

res.end('done');

@zdm
Copy link
Author

zdm commented Aug 21, 2024

Fot me hasBody is enough. I already user methos, which is limit download size.

@uNetworkingAB
Copy link
Contributor

bodyType() could return 0 for false, 1 for fix length, 2 for stream. The parser already knows all of this

@uasan
Copy link

uasan commented Aug 21, 2024

bodyType() could return 0 for false, 1 for fix length, 2 for stream. The parser already knows all of this

With a fixed body length, it is very useful to know it in advance, then it will be better like this:

req.getBodyLength(): 0 | number | Infinity

@uNetworkingAB
Copy link
Contributor

Agree, knowing the fixed length can help decide buffer size

@zdm
Copy link
Author

zdm commented Oct 19, 2024

Will you add this method to the uws or lets close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants