holepunchto / hyperdht

The DHT powering Hyperswarm
https://docs.holepunch.to
MIT License
323 stars 46 forks source link

Fix unannounce TypeError when from.id is null #161

Closed HDegroote closed 8 months ago

HDegroote commented 8 months ago

The current unannounce code assumes from.id always exists (https://github.com/holepunchto/hyperdht/blob/4148e12d26a78f88ad5b8c06b9105a644c2e1a41/lib/persistent.js#L248-L250 => TypeError when id is not a buffer)

This isn't guaranteed, occasionally data.from looks like { id: null, host: '50.50.50.50', port: 55555 } here https://github.com/holepunchto/hyperdht/blob/4148e12d26a78f88ad5b8c06b9105a644c2e1a41/index.js#L195.

This PR checks explicitly for the existence of from.id before trying to unannounce it, avoiding errors of type

.../node_modules/hyperdht/lib/persistent.js:269
  sodium.crypto_generichash_batch(hash, [
         ^

TypeError: batch element should be passed as a TypedArray
    at annSignable (...node_modules/hyperdht/lib/persistent.js:269:10)
    at signUnannounce (.../node_modules/hyperdht/lib/persistent.js:249:17)
    at HyperDHT._requestUnannounce (.../node_modules/hyperdht/index.js:444:29)
    at Query.map (.../node_modules/hyperdht/index.js:191:13)
    at Query._onvisit (.../node_modules/dht-rpc/lib/query.js:271:23)
    at IO.onmessage (.../node_modules/dht-rpc/lib/io.js:93:13)
    at UDXSocket.emit (node:events:513:28)
    at UDXSocket.emit (node:domain:489:12)
    at UDXSocket._onmessage (.../node_modules/udx-native/lib/socket.js:77:10)

Node.js v18.12.1

Note: this issue has always been there, but before we were catching all errors. Since we now use safetyCatch, the type error bubbles up and causes a crash

HDegroote commented 8 months ago

from.id being null signals that it's not a serving DHT node, so it being null here might mean that it stopped serving for some reason. It's an expected path, so just adding the if is good