Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Feature Request: Overload http.request to accept a string that is parsed by url.parse #2859

Closed
wants to merge 5 commits into from
Closed

Feature Request: Overload http.request to accept a string that is parsed by url.parse #2859

wants to merge 5 commits into from

Conversation

adammw
Copy link

@adammw adammw commented Mar 2, 2012

Mostly just for convenience. Example invocation would be:

http.get("http://www.example.com/", function(stream){ ... });

bnoordhuis
bnoordhuis reviewed Mar 2, 2012
View reviewed changes
lib/http.js
@@ -1424,6 +1425,7 @@ ClientRequest.prototype.pause = function() {


exports.request = function(options, cb) {
if (typeof options == 'string') options = URLParse(options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use url.parse() here, makes it obvious that it's an external function.

@bnoordhuis
Copy link
Member

Can you add one or two tests?

@adammw
Copy link
Author

adammw commented Mar 3, 2012

Removed URLParse and replaced with url.parse as per comments, however I'm not sure if requiring from within the function is best practice. Also added a rudimentary test based around how the others function, but again, not sure if it's done in the correct way or what cases has to be tested.

@bnoordhuis
Copy link
Member

however I'm not sure if requiring from within the function is best practice

It is not. :-) Can you move it to the module prolog? Thanks.

@bnoordhuis
Copy link
Member

@adammw Do you still want to get this merged?

@adammw
Copy link
Author

adammw commented May 1, 2012

@bnoordhuis I think it would be handy to be merged but I'd want just to check these things first:
I wasn't sure about moving url to prologue as the url variable is used in a different context inside some of the functions in http.js but it seems all of them explicitly declare url first so there shouldn't be a conflict.
Also, I am wondering if the test I created should be removed and substituted with a modified version of the test-http-url.* tests, as they test similar functionality.
Is there any documentation that would also need to be modified in this pull request?
Thanks, for checking up, I had forgotten about this patch...

@bnoordhuis
Copy link
Member

Also, I am wondering if the test I created should be removed and substituted with a modified version of the test-http-url.* tests, as they test similar functionality.

No need, the test looks fine to me.

Is there any documentation that would also need to be modified in this pull request?

Yes, please. Can you add a small blurb to doc/api/http.markdown?

@bnoordhuis
Copy link
Member

Adam, can you update the docs? I would like to land this patch.

@adammw
Copy link
Author

adammw commented May 16, 2012

Docs updated in c413c2b (6e0a8bb is optional, but I think suits the example)

@bnoordhuis
Copy link
Member

Thanks, merged with some modifications (the test wasn't working) in 4099d1e.

@bnoordhuis bnoordhuis closed this May 16, 2012
koichik added a commit that referenced this pull request Aug 24, 2012
@koichik @bnoordhuis
https: make https.get() accept a URL
752ac32 
https.get() now accepts either a URL (as a string) or an options object.

Refs #2859.
Fixes #3882.
richardlau pushed a commit to ibmruntimes/node that referenced this pull request Nov 5, 2015
@rvagg
2015-10-29, Version 5.0.0 (Stable)
837b174 
Notable changes:

* buffer: (Breaking) Removed both 'raw' and 'raws' encoding types from Buffer,
  these have been deprecated for a long time (Sakthipriyan Vairamani) nodejs#2859.
* console: (Breaking) Values reported by console.time() now have 3 decimals of
  accuracy added (Michaël Zasso) nodejs#3166.
* fs:
  - fs.readFile*(), fs.writeFile*(), and fs.appendFile*() now also accept a file
    descriptor as their first argument (Johannes Wüller) nodejs#3163.
  - (Breaking) In fs.readFile(), if an encoding is specified and the internal
    toString() fails the error is no longer thrown but is passed to the callback
    (Evan Lucas) nodejs#3485.
  - (Breaking) In fs.read() (using the fs.read(fd, length, position, encoding,
    callback) form), if the internal toString() fails the error is no longer
    thrown but is passed to the callback (Evan Lucas) nodejs#3503.
* http:
  - Fixed a bug where pipelined http requests would stall (Fedor Indutny) nodejs#3342.
  - (Breaking) When parsing HTTP, don't add duplicates of the following headers:
    Retry-After, ETag, Last-Modified, Server, Age, Expires. This is in addition
    to the following headers which already block duplicates: Content-Type,
    Content-Length, User-Agent, Referer, Host, Authorization,
    Proxy-Authorization, If-Modified-Since, If-Unmodified-Since, From, Location,
    Max-Forwards (James M Snell) nodejs#3090.
  - (Breaking) The callback argument to OutgoingMessage#setTimeout() must be a
    function or a TypeError is thrown (James M Snell) nodejs#3090.
  - (Breaking) HTTP methods and header names must now conform to the RFC 2616
    "token" rule, a list of allowed characters that excludes control characters
    and a number of separator characters. Specifically, methods and header names
    must now match /^[a-zA-Z0-9_!#$%&'*+.^`|~-]+$/ or a TypeError will be thrown
    (James M Snell) nodejs#2526.
* node:
  - (Breaking) Deprecated the _linklist module (Rich Trott) nodejs#3078.
  - (Breaking) Removed require.paths and require.registerExtension(), both had
    been previously set to throw Error when accessed
    (Sakthipriyan Vairamani) nodejs#2922.
* npm: Upgraded to version 3.3.6 from 2.14.7, see
  https://github.com/npm/npm/releases/tag/v3.3.6 for more details. This is a
  major version bump for npm and it has seen a significant amount of change.
  Please see the original npm v3.0.0 release notes for a list of major changes
  (Rebecca Turner) nodejs#3310.
* src: (Breaking) Bumped NODE_MODULE_VERSION to 47 from 46, this is necessary
  due to the V8 upgrade. Native add-ons will need to be recompiled
  (Rod Vagg) nodejs#3400.
* timers: Attempt to reuse the timer handle for setTimeout().unref(). This fixes
  a long-standing known issue where unrefed timers would perviously hold
  beforeExit open (Fedor Indutny) nodejs#3407.
* tls:
  - Added ALPN Support (Shigeki Ohtsu) nodejs#2564.
  - TLS options can now be passed in an object to createSecurePair()
    (Коренберг Марк) nodejs#2441.
  - (Breaking) The default minimum DH key size for tls.connect() is now 1024
    bits and a warning is shown when DH key size is less than 2048 bits. This a security consideration to prevent "logjam" attacks. A new minDHSize TLS
    option can be used to override the default. (Shigeki Ohtsu) nodejs#1831.
* util:
  - (Breaking) util.p() was deprecated for years, and has now been removed
    (Wyatt Preul) nodejs#3432.
  - (Breaking) util.inherits() can now work with ES6 classes. This is considered
    a breaking change because of potential subtle side-effects caused by a
    change from directly reassigning the prototype of the constructor using
    `ctor.prototype = Object.create(superCtor.prototype, { constructor: { ... } })`
    to using `Object.setPrototypeOf(ctor.prototype, superCtor.prototype)`
    (Michaël Zasso) nodejs#3455.
* v8: (Breaking) Upgraded to 4.6.85.25 from 4.5.103.35 (Ali Ijaz Sheikh) nodejs#3351.
  - Implements the spread operator, see
    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_operator
    for further information.
  - Implements new.target, see
    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/new.target
    for further information.
* zlib: Decompression now throws on truncated input (e.g. unexpected end of
  file) (Yuval Brik) nodejs#2595.

PR-URL: nodejs/node#3466
richardlau pushed a commit to ibmruntimes/node that referenced this pull request Nov 5, 2015
@rvagg
2015-10-29, Version 5.0.0 (Stable)
384e6c2 
Notable changes:

* buffer: (Breaking) Removed both 'raw' and 'raws' encoding types from Buffer,
  these have been deprecated for a long time (Sakthipriyan Vairamani) nodejs#2859.
* console: (Breaking) Values reported by console.time() now have 3 decimals of
  accuracy added (Michaël Zasso) nodejs#3166.
* fs:
  - fs.readFile*(), fs.writeFile*(), and fs.appendFile*() now also accept a file
    descriptor as their first argument (Johannes Wüller) nodejs#3163.
  - (Breaking) In fs.readFile(), if an encoding is specified and the internal
    toString() fails the error is no longer thrown but is passed to the callback
    (Evan Lucas) nodejs#3485.
  - (Breaking) In fs.read() (using the fs.read(fd, length, position, encoding,
    callback) form), if the internal toString() fails the error is no longer
    thrown but is passed to the callback (Evan Lucas) nodejs#3503.
* http:
  - Fixed a bug where pipelined http requests would stall (Fedor Indutny) nodejs#3342.
  - (Breaking) When parsing HTTP, don't add duplicates of the following headers:
    Retry-After, ETag, Last-Modified, Server, Age, Expires. This is in addition
    to the following headers which already block duplicates: Content-Type,
    Content-Length, User-Agent, Referer, Host, Authorization,
    Proxy-Authorization, If-Modified-Since, If-Unmodified-Since, From, Location,
    Max-Forwards (James M Snell) nodejs#3090.
  - (Breaking) The callback argument to OutgoingMessage#setTimeout() must be a
    function or a TypeError is thrown (James M Snell) nodejs#3090.
  - (Breaking) HTTP methods and header names must now conform to the RFC 2616
    "token" rule, a list of allowed characters that excludes control characters
    and a number of separator characters. Specifically, methods and header names
    must now match /^[a-zA-Z0-9_!#$%&'*+.^`|~-]+$/ or a TypeError will be thrown
    (James M Snell) nodejs#2526.
* node:
  - (Breaking) Deprecated the _linklist module (Rich Trott) nodejs#3078.
  - (Breaking) Removed require.paths and require.registerExtension(), both had
    been previously set to throw Error when accessed
    (Sakthipriyan Vairamani) nodejs#2922.
* npm: Upgraded to version 3.3.6 from 2.14.7, see
  https://github.com/npm/npm/releases/tag/v3.3.6 for more details. This is a
  major version bump for npm and it has seen a significant amount of change.
  Please see the original npm v3.0.0 release notes for a list of major changes
  (Rebecca Turner) nodejs#3310.
* src: (Breaking) Bumped NODE_MODULE_VERSION to 47 from 46, this is necessary
  due to the V8 upgrade. Native add-ons will need to be recompiled
  (Rod Vagg) nodejs#3400.
* timers: Attempt to reuse the timer handle for setTimeout().unref(). This fixes
  a long-standing known issue where unrefed timers would perviously hold
  beforeExit open (Fedor Indutny) nodejs#3407.
* tls:
  - Added ALPN Support (Shigeki Ohtsu) nodejs#2564.
  - TLS options can now be passed in an object to createSecurePair()
    (Коренберг Марк) nodejs#2441.
  - (Breaking) The default minimum DH key size for tls.connect() is now 1024
    bits and a warning is shown when DH key size is less than 2048 bits. This a security consideration to prevent "logjam" attacks. A new minDHSize TLS
    option can be used to override the default. (Shigeki Ohtsu) nodejs#1831.
* util:
  - (Breaking) util.p() was deprecated for years, and has now been removed
    (Wyatt Preul) nodejs#3432.
  - (Breaking) util.inherits() can now work with ES6 classes. This is considered
    a breaking change because of potential subtle side-effects caused by a
    change from directly reassigning the prototype of the constructor using
    `ctor.prototype = Object.create(superCtor.prototype, { constructor: { ... } })`
    to using `Object.setPrototypeOf(ctor.prototype, superCtor.prototype)`
    (Michaël Zasso) nodejs#3455.
* v8: (Breaking) Upgraded to 4.6.85.25 from 4.5.103.35 (Ali Ijaz Sheikh) nodejs#3351.
  - Implements the spread operator, see
    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_operator
    for further information.
  - Implements new.target, see
    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/new.target
    for further information.
* zlib: Decompression now throws on truncated input (e.g. unexpected end of
  file) (Yuval Brik) nodejs#2595.

PR-URL: nodejs/node#3466
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
feature-request http
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adammw @bnoordhuis