Skip to content

Handle & report server request errors as 'requestError' events#57

Merged
lsongdev merged 2 commits intolsongdev:masterfrom
pimterry:handle-server-errors
Dec 14, 2021
Merged

Handle & report server request errors as 'requestError' events#57
lsongdev merged 2 commits intolsongdev:masterfrom
pimterry:handle-server-errors

Conversation

@pimterry
Copy link
Contributor

Fixes #42.

As discussed in that issue, without this change any invalid DNS packet can crash the server, e.g. by sending an empty UDP packet.

With this change, it's reported as a requestError event (not an error event, so implementers can ignore this without crashing Node.js if they choose to) and the request is safely ignored.

The changeset for DoH may look large, but that's just due to indentation changes - disable whitespace diffs to see the real change.

It might be possible to improve on this by:

  • Improving the parser error messages for various relevant cases, to more easily debug issues
  • Sending a useful error response back to the client

I don't think either are especially urgent though, whereas it's quite important to ensure that the server doesn't crash completely when sent trivially invalid input.

@lsongdev lsongdev merged commit b2e994c into lsongdev:master Dec 14, 2021
@pimterry pimterry deleted the handle-server-errors branch December 14, 2021 16:06
@pimterry
Copy link
Contributor Author

Amazing, thanks for merging all these PRs so quickly! 👍

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

Successfully merging this pull request may close these issues.

Diagnostic tool can be used to kill the UDP listener

2 participants