Polyfill dns_check_record(), dns_get_record(), and dns_get_mx()

[adamziel]

Closes #1042 by polyfilling the dns_* functions that are missing in the WebAssembly
build of PHP. In the PHP source code, these functions are included
conditionally when the HAVE_DNS_SEARCH_FUNC constant is true, which it
isn't in the Emscripten build.

The polyfills return either false or an empty array and do not
actually perform any DNS checks.

In Node.js, we could support actual dns checks. Let's leave that for a
subsequent iteration. In the browser, there isn't much we can do.

Remaining work

Rebuild all the PHP versions or else the CI will fail

Testing instructions

Confirm the CI checks pass

CC @p-jackson @sejas @wojtekn @artpi

[p-jackson]

Great to have a fix built right into php-wasm 😊

I'm just pondering here, I could be wrong, but are these polyfills almost too graceful? dns_get_record( 'google.com' ) will return an empty array and the calling code has no idea nothing happened. We could return false to indicate an error, but I'm worried that might break plugins that aren't handling errors correctly (they never needed to anticipate a situation where google.com has no DNS records).

Would logging a warning be appropriate? Something like WordPress's _doing_it_wrong() function. If a plugin isn't working in Playground, a log would give the plugin author a hint to go on.

[adamziel]

Would logging a warning be appropriate? Something like WordPress's _doing_it_wrong() function. If a plugin isn't working in Playground, a log would give the plugin author a hint to go on.

Good thinking! I added a PHP warning to those functions, I'll just need to adjust the tests to expect that warning.

[adamziel]

@p-jackson I may need to sidetrack the remaining bit of the work in this PR. Would you be up for taking over so we could get it in?

[p-jackson]

I'll see what I can do 😅🤞
Can't guarantee when I'll get to it.

[bgrgicak]

Almost done. I just need to find the right place to add DNS constants so they are always available, but not duplicated.

[bgrgicak]

@adamziel this should now be ready for a review

[p-jackson]

Thanks for adding the messages 👍 I think it's much kinder to devs than pretending everything worked fine 😊

I tested it manually using this little script

const { NodePHP } = require('./dist/packages/php-wasm/node');

NodePHP.load('8.0')
   .then((php) => php.run({ code: '<?php dns_get_record("google.com");'}) )
   .then((output) => console.log(output.text));

I noticed that the message is incorrect if you call an alias e.g. when calling getmxrr() the warning refers to dns_get_mx. But I think that's fine, just wanted to note it here.

[p-jackson]

#includeing a *.c file seems a bit unorthodox. Doesn't this leave us open to having issues with double definitions down the track?

I feel it would be more typical to ensure dns_polyfill.c gets compiled separately and then linked into the final output? i.e. including it in the build command here:

wordpress-playground/packages/php-wasm/compile/php/Dockerfile

Line 932 in 3525045

/root/php_wasm.c \

We'd need another header file to declare the functions so they can be included in the additional_functions array. But still, that seems a bit more conventional.

But I might be missing something. Was there a reason why this needed to be included as a C file? Could the DNS functions simply be added to the giant php_wasm.c file?