search

TuxInvader / nginx-dns

Sample Configuration for DNS over HTTPS (DoH/DoT gateway) and GSLB with NGINX
BSD 2-Clause "Simplified" License
194 stars 47 forks source link

Update dns.js and libdns.js to support njs 8.0.0 #20

Open mariamatthews opened 1 year ago

mariamatthews commented 1 year ago

Hi @TuxInvader ! Hope you're doing well :) It's me again lol

In njs 8.0.0 two methods used by the code were deprecated:

*) Change: non-compliant deprecated String methods were removed.
    The following methods were removed: String.bytesFrom(),
    String.prototype.fromBytes(), String.prototype.fromUTF8(),
    String.prototype.toBytes(), String.prototype.toUTF8(),
    String.prototype.toString(encoding).

I have updated the code in dns.js and libdns.js to use native Buffer.from function to allow for latest njs 8.0.0 support. :)

michaelkkehoe commented 1 year ago

This fixed the issues I was having.

mariamatthews commented 1 year ago

@michaelkkehoe good to know! 😄

TuxInvader commented 1 year ago

Hi Maria,

Apologies for the delay in looking at this. I already had a branch (v2) that was using buffers in testing. I have merged that now, but it seems you also fixed an issue with GET requests. Does this resolve an issue with a particular client?

mariamatthews commented 1 year ago

Hi @TuxInvader ,

No worries, thanks for checking in :) I believe this was for Chrome/Brave browsers - when trying to point them to the Nginx DNS endpoint they were not validating it correctly and/or displaying an error and that change was correcting the issue.

I feel like I could potentially re-test this with your new v2?

TuxInvader commented 1 year ago

Hi @mariamatthews

I feel like I could potentially re-test this with your new v2?

That would be awesome if you could, and then just create a PR for the GET changes if still required.

Thank you :-)

honigpferd commented 8 months ago

current master works fine here for me.

This PR is probably obsolete now. wanna close it?