daemon/http: HTTP response codes

Fixes #728 (closed)

Related #618 (closed)

Currently:

• Replies with 400 when a GET query has no DNS query in it (the basic case in #728 (closed))
• Replies with 404 when querying unknown endpoints (anything other than /doh or /dns-query)
• Replies with 431 when a header is too large

To do:

• Reply with 400 when the POST body is malformed (but only if DNS itself cannot deal with it, then it is 200 as per RFC 8484)
• Reply with 413 when the POST body is too long (attempted but not working at the moment) (changed to 400)
• Reply with 414 when the GET URL is too long (changed to 400)
• Check RFCs for more codes
• Make sure the way it's currently simply calling http_send_response on error does not cause any problems
• Possibly more?

changed the description

marked the checklist item Check RFCs for more codes as completed

added 1 commit

• 5b15050e - fixup! daemon/http: add basic HTTP response codes

Compare with previous version

added 1 commit

• 22217915 - daemon/http: return 400 on stream end with no processed packets

Compare with previous version

added 1 commit

• 281f9cad - daemon/http: return 400 on failed packet_parse + improved stream handling

Compare with previous version

marked the checklist item Reply with 400 when the POST body is malformed (but only if DNS itself cannot deal with it, then it is 200 as per RFC 8484) as completed

added 1 commit

• f1509cbb - fixup! daemon/http: return 400 on failed packet_parse + improved stream handling

Compare with previous version

added 9 commits

• f1509cbb...d1988269 - 6 commits from branch master
• 1c453b61 - daemon/http: add basic HTTP response codes
• 22d95750 - daemon/http: return 400 on stream end with no processed packets
• c7abc86a - daemon/http: return 400 on failed packet_parse + improved stream handling

Compare with previous version

The Lua config "too long POST" test currently has issues with its connection timing out. I'm not sure what that is about, both curl and PowerShell's Invoke-RestMethod seem to react as intended to the response code (tested with similar HTTP body data as present in the test). I have commented that test out for now.

I can try investigating further, but I have spent a lot of time debugging this and still haven't found what the problem is.

I have changed the "too long POST" and "too long GET" response codes to the generic 400. In the worker, I can easily find out that no packets were successfully parsed from the request, but not why.

Also, as per RFC 2616, I think that the 413 and 414 response codes are meant to indicate that the HTTP server itself is unwilling/unable to process long requests, with 414 even being called rare. Since the length of a packet is not being checked by the HTTP part of the resolver, but by libknot, the 400 response code might actually be more appropriate.

For unsupported HTTP methods, I have changed the response from 405 to 501. I think it is more appropriate, as our HTTP server itself does not implement any methods other than GET, POST, and newly HEAD (as it is required by RFC 2616) anywhere. The 405 response is also required to include an Allow header, while 501 is not.

added 1 commit

• 05d1c744 - fixup! daemon/http: return 400 on failed packet_parse + improved stream handling

Compare with previous version

changed the description

added 2 commits

• 51e1f428 - daemon/http: return 400 on failed packet_parse + improved stream handling
• 81b88ca4 - daemon/http: move status sends outside nghttp2 callbacks

Compare with previous version

marked the checklist item Make sure the way it's currently simply calling http_send_response on error does not cause any problems as completed

marked the checklist item Possibly more? as completed

marked this merge request as ready