Get method does not check options's Object prototype

[mihai1voicescu]

• Version: v6.10.0
• Platform: 3.10.0-514.2.2.el7.x86_64 deps: update openssl to 1.0.1j #1 SMP Tue Dec 6 23:06:41 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux AND Windows 8.1 64

The get method of http and https modules does not check the prototype of the given object. I guess this replicates to all other request/response methods.
The following code is ran on a machine that does not have anything running on port 8989:

'use strict';

const http = require("http");

let options = {
        port: 8989
};
let optionsChild = Object.create(options);

http.get(options, function () {
        console.log("Success options");
}).on('error', console.error);
http.get(optionsChild, function () {
        console.log("Success optionsChild");
}).on('error', console.error);

And we receive the following output:

{ Error: connect ECONNREFUSED 127.0.0.1:8989
    at Object.exports._errnoException (util.js:1022:11)
    at exports._exceptionWithHostPort (util.js:1045:20)
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1087:14)
  code: 'ECONNREFUSED',
  errno: 'ECONNREFUSED',
  syscall: 'connect',
  address: '127.0.0.1',
  port: 8989 }
Success optionsChild

This means the port takes the default value and successfully connects instead of taking the 8989 port from the prototype.

1. Is this intended?

The need for this code arose from the necessity to reuse some default options in multiple requests. So I would like to ask a followup question:

2. Can reusing(altering) the options Object passed to a http/https request cause unexpected behavior(requests being made to the last value of the object instead of the value at the moment of the function call or performance losses)?

[cjihrig]

The issue is that, under the hood, http.get() copies the options provided by the user using util._extend(). util._extend() copies properties using Object.keys(). Object.keys() doesn't include the prototype.

[seishun]

Last time this was discussed it led to nowhere: nodejs/node-v0.x-archive#7587 (FWIW, my opinion on it hasn't changed)

But I agree it should at least be documented.

[raphaelokon]

@seishun I agree on the documentation bit.

[raphaelokon]

I think this issue is just a mod of this one here: nodejs/node-v0.x-archive#7587 (comment)

[mihai1voicescu]

Thank you for the quick response!
Indeed the documentation should at least specify that.

Regarding the second question. If _extend is used then a shallow copy is made. Any alteration to the properties not on the first level will impact the first request if the object is used after a asynchronous call. Is this the case?

[joyeecheung]

Added doc and good first contribution labels. Feel free to send in a PR mentioning this behavior in doc/http.md :)