web-frameworks icon indicating copy to clipboard operation
web-frameworks copied to clipboard

Are we testing real-world frameworks?

Open trikko opened this issue 11 months ago • 23 comments

I believe that the frameworks included here should at least satisfy a minimum level of adherence to the HTTP standard. I'm not saying they need to be perfect, but they should at least implement the basics of the standard.

Otherwise, this isn't a benchmark of HTTP frameworks, but rather a benchmark of pseudo-HTTP programs designed to win benchmarks, because obviously the fewer checks you perform, the faster you are.

And in that case, they wouldn't even be useful in real-life scenarios, would they?

Give it a try and run this script against any random web framework, and try to check the responses they give. It's just a little subset of what should be tested.

#!/bin/bash

echo "This should be rejected with a 400 (sdsP/4.3 is not a valid protocol)"
echo "----------------------------------"
echo -e "GET /#/pizza/user/100 sdsP/4.3\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "This should be rejected with a 400 (HTTP/1.3 is not a valid protocol)"
echo "----------------------------------"
echo -e "GET /#/pizza/user/100 HTTP/1.3\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "This should be rejected with a 400 (HTTP/0.2 is not a valid protocol)"
echo "----------------------------------"
echo -e "GET /#/pizza/user/100 HTTP/0.2\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "Server should respond with HTTP/1.0 200, not HTTP/1.1 200"
echo "----------------------------------"
echo -e "GET / HTTP/1.0\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "GETTY is not a valid method, 400 should be returned"
echo "----------------------------------"
echo -e "GETTY / HTTP/1.0\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "PET is not a valid method, 400 should be returned"
echo "----------------------------------"
echo -e "PET / HTTP/1.0\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "GIT is not a valid method, 400 should be returned"
echo "----------------------------------"
echo -e "GIT / HTTP/1.0\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "Invalid headers, 400 should be returned"
echo "----------------------------------"
echo -e "GET / HTTP/1.0\r\nconnection: close\r\ninvalid header\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

echo "Header with no space between colon and value are still valid. This request should be closed by the server immediately"
echo "----------------------------------"
echo -e "GET / HTTP/1.1\r\nhost:localhost\r\nconnection:close\r\n\r\n" | nc -w 10 localhost 3000 ; echo ; echo


echo "Every http/1.1 response must include date and (content-length or transfer-encoding)"
echo "----------------------------------"
echo -e "GET / HTTP/1.1\r\nhost:localhost\r\nconnection: close\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo
echo -e "GET /user/1009 HTTP/1.1\r\nconnection: close\r\nhost:localhost\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo
echo -e "ààèèììòùù\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo


echo "This is not a valid http/1.0 request, 400 should be returned"
echo "----------------------------------"
echo -e "GET HTTP/1.0\r\n\r\n" | nc -w 3 localhost 3000 ; echo ; echo

trikko avatar Jan 03 '25 15:01 trikko

You say that we are not testing any real world usage, I cannot disagree.

The scenario are not very useful , we are testing the routing part of a piece of software. There is an open discussion for that (I mean to have a new scenario) https://github.com/the-benchmarker/web-frameworks/discussions/8093

Also we need to change our load tool in order to test at constant rate, there is also an open discussion. The idea is to use a tool like Vegeta to make http calls, instead of wrk to make sure we are more realistic in figures.

Also what you point is that we need to check http headers ... I agree that it could be accurate to test http communication facing some security rules, like OWASP. You script cool be cool and can be used in a one shot manner to see if there is some framework that are falsey, and open an issue on their repository.

waghanza avatar Jan 04 '25 07:01 waghanza

I mean that your point @trikko is more related to HTTP part, and that is more the responsibility of HTTP stack, not the whole framework.

For example, node or deno or bun for JavaScript should pass this kind of test, and this will affect express, bunicorn, fastify ... but the root cause is not in their scope

waghanza avatar Jan 04 '25 07:01 waghanza

After some verifications, it appears that echo -e "GET /#/pizza/user/100 sdsP/4.3\r\nconnection: close\r\n\r\n" | nc -w 3 $1 3000 returns various kind of things :

  • HTTP/1.1 for veb, fastify, chubbyts, hunt cc @enghitalo @mcollina @dominikzogg @zoujiaqing
  • Nothing for httpbeast, falco , serverino cc @dom96 @pimbrouwers @trikko (server closes connection ???)
  • A 400 for hono with (bun and node) and elysia cc @yusukebe @SaltyAom

waghanza avatar Jan 04 '25 09:01 waghanza

It was related to http stack just because it was easier to test!

There would be many similar tests, for example, "expect: 100-continue." Surely, if the server does not respond correctly, it has probably never been widely used in real life, because even a simple file upload with curl triggers this. And if not handled, the server likely remains waiting indefinitely.

This should be a strict requirement. 🙂

trikko avatar Jan 04 '25 09:01 trikko

400 means bad (http) request. Is that a http request at all? Even closing connection I think is fine in this case.

There's nothing indicates it's a http request and for sure not a http/1.1! "sdsP" could be a completely different protocol.

For example: https://www.twilio.com/docs/glossary/sip-invites#example-sip-invite

In this case should I reply with HTTP?

trikko avatar Jan 04 '25 09:01 trikko

Not sure I'm legitimate to answer this 😛

The idea, imho, is more to stick to the standard, but again not in the scope of this project.

waghanza avatar Jan 04 '25 09:01 waghanza

I think it is related. Ignoring lines and just read the first G of "GET" and the URL before the last space is a huge advantage in benchmark for speed, but not fair.

At least return 400, not 200 :)

Same for http/1.0 Vs http/1.1 . If someone askS you a page with http/1.0 you have to reply with 1.0protocol not with something else just to be faster on benchmark.

If this is the point one could write a fast server that just search for /user or /user/ in the first line and reply with the answer you expect

Another example: date header on http/1.1 response is mandatory. And it's a bit consuming to build that extra header with current date in that format for each request. Not fair to skip it to be faster. So on.

Il sab 4 gen 2025, 10:53 Marwan Rabbâa @.***> ha scritto:

Not sure I'm legitimate to answer this 😛

The idea, imho, is more to stick to the standard, but again not in the scope of this project.

— Reply to this email directly, view it on GitHub https://github.com/the-benchmarker/web-frameworks/issues/8116#issuecomment-2571045981, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE575LPEF2XPE6NUDM3B6L2I6VQDAVCNFSM6AAAAABUR5AGNGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNZRGA2DKOJYGE . You are receiving this because you were mentioned.Message ID: @.***>

trikko avatar Jan 04 '25 10:01 trikko

You have some points 😛 I plan to have OWASP tests here, to test the implementations.

I can some test also to make sure that a server does not return HTTP1.1 if we send HTTP/1.0, but this is more like for investigation, not something we can merge

waghanza avatar Jan 04 '25 10:01 waghanza

I think I could write a small tool to make some tests, it would be nice to see which % is passed by each server!

IMHO some tests must be mandatory (parsing of http protocol, correct reply to "connection: xxx", etc.), and every server must at least pass x% of non-mandatory tests

It would be nice to have a column with test passed (%)

trikko avatar Jan 04 '25 12:01 trikko

I think you should start a PR / issue to discuss with communities, but not something we should merge / add to the UI.

imho, this is usefull to make some network handling part more reliable facing http standard

you could even write a list of tests (a plain list of bullet points), I can convert them into ruby for specs here

waghanza avatar Jan 04 '25 12:01 waghanza

@waghanza

  • chubbyts with node bridge: https://github.com/chubbyts/chubbyts-http-node-bridge/blob/master/src/node-http.ts#L82 => Node JS built in http server logic.
  • chubbyts with uwebsockets bridge: https://github.com/chubbyts/chubbyts-http-uwebsockets-bridge/blob/master/src/uwebsocket-http.ts#L87 => Faked 1.1, i try to challenge that.

dominikzogg avatar Jan 04 '25 22:01 dominikzogg

@waghanza it seems to be not possible to read the httpVersion of a ubwesocket.js request.

dominikzogg avatar Jan 04 '25 22:01 dominikzogg

Maybe an issue with nodejs then for chubbyts @dominikzogg

waghanza avatar Jan 05 '25 07:01 waghanza

I released a small tool with some tests. Try it: https://github.com/trikko/http-validator/

Just run dub when a server is running

trikko avatar Jan 10 '25 22:01 trikko

For now, it's just a few tests. But apparently, they're enough to cause several servers to crash catastrophically (and many others fail!).

trikko avatar Jan 10 '25 23:01 trikko

Some servers are made only win benchmarks :) not to be used in real solutions :P

cyrusmsk avatar Jan 11 '25 13:01 cyrusmsk

And some framework used here and widely used in production are not fully respecting http standards

waghanza avatar Jan 11 '25 14:01 waghanza

Yes but at least I hope they respect the bare minimum and they don't crash just for a request they received.

trikko avatar Jan 11 '25 14:01 trikko

Fun fact. I see a reply to this request (4 bytes) with 200 ok (I wonder which uri...):

"\r\n\r\n"

trikko avatar Jan 12 '25 09:01 trikko

Yeah, I'll open a branch to check that and a PR to ping all buddies concerned by that

waghanza avatar Jan 12 '25 12:01 waghanza

Another interesting thing: most servers don't seem to check the size of posts.

So I can make a post with a 100TB file by default on a server without it even flinching. There should be a default limit (adjustable, of course). Leaving such an open-ended limit is very dangerous.

trikko avatar Jan 12 '25 18:01 trikko

So GET /user/123 must return 123, what about GET /user/asd? "asd" even if it's not a number? Writing a test for it :)

trikko avatar Jan 12 '25 18:01 trikko

If GET /user/asd is valid, it would be nice to test some other things.

For example:

  • /user/hello%20world must print "hello world"
  • /user/hello/world must fail but
  • /user/hello%2Fworld must print "hello/world"

If you want only /user/:number: all of these example must fail (and we can't test if decoding works properly)

trikko avatar Jan 12 '25 20:01 trikko