Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The document is different from the runtime #48688

Closed
an5er opened this issue Jul 7, 2023 · 5 comments
Closed

The document is different from the runtime #48688

an5er opened this issue Jul 7, 2023 · 5 comments
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem.

Comments

@an5er
Copy link
Contributor

an5er commented Jul 7, 2023

Version

all

Platform

all

Subsystem

No response

What steps will reproduce the bug?

that's what it says in the documentation

but i test it

How often does it reproduce? Is there a required condition?

all

What is the expected behavior? Why is that the expected behavior?

It does not inherit the properties of the prototype

What do you see instead?

It inherits the properties of the prototype

Additional information

No response

@lpinca
Copy link
Member

lpinca commented Jul 7, 2023

The "with method always set to GET." part is wrong but the other sentence is correct. The following example is what the documentation refers to.

const assert = require('assert');
const http = require('http');

function Options() {}

Options.prototype.method = 'POST';

const options = new Options();

assert.strictEqual(options.method, 'POST');

const server = http.createServer();

server.on('request', function (request, response) {
  console.log(request.method);
  response.end('OK');
});

server.listen(function () {
  const { port } = server.address();
  const request = http.get(`http://localhost:${port}`, options);

  request.on('response', function (response) {
    response.resume();
    response.on('end', function () {
      server.close();
    });
  });
});

@an5er
Copy link
Contributor Author

an5er commented Jul 7, 2023

The last sentence inherits attributes from the prototype to ignore ambiguities, which I understand as follows:

const http = require('http');

var obj = {};

obj.__proto__.method = 'POST';

const server = http.createServer();

server.on('request', function (request, response) {
    console.log(request.method);
    response.end('OK');
});

server.listen(function () {
    const { port } = server.address();
    const request = http.get(`http://localhost:${port}`);

    request.on('response', function (response) {
        response.resume();
        response.on('end', function () {
            server.close();
        });
    });
});

If the code written by the user has any prototype chain contamination, there may be a vulnerability hazard

@benjamingr
Copy link
Member

Wanna open a docs PR? A fix would probably be: "With the method set to GET by default" or something?

@benjamingr benjamingr added http Issues or PRs related to the http subsystem. doc Issues and PRs related to the documentations. labels Jul 7, 2023
@an5er
Copy link
Contributor Author

an5er commented Jul 7, 2023

Yes, I did, I originally wanted to fix this prototype inheritance issue because it could cause bugs, but I found I couldn't do it, so I fixed docs

@lpinca
Copy link
Member

lpinca commented Jul 7, 2023

If the code written by the user has any prototype chain contamination, there may be a vulnerability hazard

Changing the global object prototype is a different thing and is at the user's risk. It is not limited to this but the whole runtime environment.

@lpinca lpinca closed this as completed in 8610be7 Jul 12, 2023
juanarbol pushed a commit that referenced this issue Jul 13, 2023
PR-URL: #48692
Fixes: #48688
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48692
Fixes: nodejs#48688
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48692
Fixes: nodejs#48688
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 12, 2023
PR-URL: #48692
Fixes: #48688
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 13, 2023
PR-URL: #48692
Fixes: #48688
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 17, 2023
PR-URL: #48692
Fixes: #48688
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants