feat: add support for localaddress

[metcoder95]

This relates to...

• Implement localAddress in Agent #1578

Rationale

Changes

• feat: add support for localaddress

Features

• Adds support for localAddress by forwarding the option to the net.Socket#connect method.

Bug Fixes

Breaking Changes and Deprecations

Status

KEY: S = Skipped, x = complete

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[codecov-commenter]

Codecov Report

Base: 94.89% // Head: 94.19% // Decreases project coverage by -0.70% ⚠️

Coverage data is based on head (82c2025) compared to base (e5e5b97).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1659      +/-   ##
==========================================
- Coverage   94.89%   94.19%   -0.71%     
==========================================
  Files          53       53              
  Lines        4803     4960     +157     
==========================================
+ Hits         4558     4672     +114     
- Misses        245      288      +43     

Impacted Files	Coverage Δ
lib/core/symbols.js	100.00% <ø> (ø)
lib/client.js	97.64% <100.00%> (+<0.01%)	⬆️
lib/core/connect.js	100.00% <100.00%> (ø)
lib/fetch/dataURL.js	88.05% <0.00%> (-8.48%)	⬇️
lib/fetch/constants.js	96.29% <0.00%> (-3.71%)	⬇️
lib/fetch/webidl.js	94.62% <0.00%> (-3.69%)	⬇️
lib/fetch/file.js	90.81% <0.00%> (-2.13%)	⬇️
lib/fetch/body.js	95.90% <0.00%> (-1.21%)	⬇️
lib/fetch/util.js	85.16% <0.00%> (-1.14%)	⬇️
lib/fetch/index.js	83.36% <0.00%> (-0.74%)	⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[metcoder95]

The PR is overall ready for review, I'm having a few struggles trying to test it as by default the socket.localAddress is 127.0.0.1.
I tried with a DNS lookup to see if IPV6 is available and use it instead, but I'm facing errors on Undici when trying to do so.

Any good hint about how to test this properly? 🙂

[mcollina]

what error are you facing?
How is node.js testing for this feature?

[metcoder95]

`EINVAL ::1` for IPV6

✖ bind EINVAL ::1

test: basic connect with localaddress
at:
line: 919
column: 18
file: node:net
function: internalConnect

I can keep it with 127.0.0.1 but that wouldn't really be testing the implementation as this is the default address use when doing socket.connect.

How is node.js testing for this feature?

Didn't check that yet, I'll do that tomorrow 👍

[mcollina]

I can keep it with 127.0.0.1 but that wouldn't really be testing the implementation as this is the default address use when doing socket.connect.

Is it? localhost can be resolved to either ::1 or 127.0.0.1.

[metcoder95]

I can keep it with 127.0.0.1 but that wouldn't really be testing the implementation as this is the default address use when doing socket.connect.

Is it? localhost can be resolved to either ::1 or 127.0.0.1.

Yes, at least at my machine. The socket is automatically resolving to 127.0.0.1 and when trying to force the switch to ::1, I automatically have the EINVAL error.

Sample

`Ignore the DNS, I was playing around to see if maybe the IPV6 was not being resolved locally`

test('basic connect with localaddress', { only: true }, (t) => {
  t.plan(4)

  const server = http.createServer((c) => {
    t.fail()
  })
  server.on('connect', (req, socket, firstBodyChunk) => {
    socket.write('HTTP/1.1 200 Connection established\r\n\r\n')

    let data = firstBodyChunk.toString()
    socket.on('data', (buf) => {
      data += buf.toString()
    })

    socket.on('end', () => {
      socket.end(data)
    })
  })
  t.teardown(server.close.bind(server))

  dns.lookup('localhost', { family: 6 }, (err, address) => {
    if (err) t.fail(err)

    server.listen(0, async () => {
      const client = new Client(`http://localhost:${server.address().port}`, {
        // localAddress: '127.0.0.1'
      })
      t.teardown(client.close.bind(client))

      const signal = new EE()
      const promise = client.connect({
        signal,
        path: '/'
      })
      t.equal(signal.listenerCount('abort'), 1)
      const { socket } = await promise
      t.equal(signal.listenerCount('abort'), 0)
      t.equal(socket.localAddress, 'wrong-input')

      let recvData = ''
      socket.on('data', (d) => {
        recvData += d
      })

      socket.on('end', () => {
        t.equal(recvData.toString(), 'Body')
      })

      socket.write('Body')
      socket.end()
    })
  })
})

[mcollina]

Are you sure there is an IPv6 network on your machine?

[metcoder95]

It's enabled and set default to automatic. Not in use right now, but ::1 is properly linked to localhost.

So, not 100% how to test it

[mcollina]

Why did you remove this?

[metcoder95]

Finger error, brought back in 82c2025 🙂

[mcollina]

lgtm