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

doc: http.ClientRequest path property description needs clarification #25864

Closed
mscdex opened this issue Feb 1, 2019 · 3 comments · Fixed by #26259
Closed

doc: http.ClientRequest path property description needs clarification #25864

mscdex opened this issue Feb 1, 2019 · 3 comments · Fixed by #26259
Assignees
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem.

Comments

@mscdex
Copy link
Contributor

mscdex commented Feb 1, 2019

  • Version: master
  • Platform: n/a
  • Subsystem: doc

The documentation for request.path mentions that the property is read-only, which is misleading.

First, it is technically not set as a real read-only property, so it is mutable. Secondly, request.path is read from at a later point in time if headers was not passed to the ClientRequest constructor.

Here is a small example that shows this:

const http = require('http');

http.createServer(function(req, res) {
  this.close();
  console.log('req.url =', req.url);
  res.end();
}).listen(0, function() {
  const req = http.request(
    { port: this.address().port, method: 'GET' },
    (res) => res.resume()
  );
  console.log('req.path =', req.path);
  req.path = '/foo/bar/baz';
  req.end();
});

// prints:
// req.path = /
// req.url = /foo/bar/baz

I would suggest removing the 'Read-only' portion from the description or making the property truly read-only using the appropriate property descriptor.

@mscdex mscdex added http Issues or PRs related to the http subsystem. doc Issues and PRs related to the documentations. labels Feb 1, 2019
@vsemozhetbyt
Copy link
Contributor

Refs: #25788

@JungMinu JungMinu self-assigned this Feb 1, 2019
@JungMinu
Copy link
Member

JungMinu commented Feb 1, 2019

I will take care of this issue

foobarboobah added a commit to foobarboobah/node that referenced this issue Feb 6, 2019
@JungMinu
Copy link
Member

Fixed in 2e125cc

addaleax pushed a commit that referenced this issue Mar 1, 2019
Clarify `http.ClientRequest` path description.

Fixes: #25864
PR-URL: #26259
Reviewed-By: Luigi Pinca <luigipinca@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

Successfully merging a pull request may close this issue.

3 participants