#515, #2894 Strip port suffix from Host header if the protocol is known.#2904
#515, #2894 Strip port suffix from Host header if the protocol is known.#2904mikeal merged 5 commits intorequest:masterfrom paambaati:master
Conversation
This partially revert ff6d6c6, and still works for IPv6 addresses as well.
|
Tests seem to be failing because of the travis-ci/travis-ci#8361 (comment) proposes a possible fix, but would require using Further reading - https://docs.travis-ci.com/user/reference/overview/#Virtualisation-Environment-vs-Operating-System |
|
@mikeal @simov This is also very similar to the recent NPM 418 errors discussed here and here - npm/npm#20791 |
|
This must be a regression, we used to take care of this by using the property from url.parse that doesn't include the header on known protocols. Any idea when this might have happened? Can you disable the IPv6 test since it doesn't work in Travis? |
|
@mikeal I just checked, and the last CI runs are all green.
From what I can tell, this happened in #2571, here - https://github.com/request/request/pull/2571/files#diff-ccc0734f65dd7a299409ff07d35be095L293 |
|
Ah, thanks, too bad that regression snuck in :( |
|
Sorry, guys! |
…rotocol is known. (request#2904) * Strip port suffix from Host header if the protocol is known. This partially revert ff6d6c6, and still works for IPv6 addresses as well. * Port is a string out of url.parse(). See https://nodejs.org/api/url.html#url_url_port * Port is a string out of url.parse(). See https://nodejs.org/api/url.html#url_url_port * Add tests for the new Host header changes.
@mikeal This PR should fix #515 and #2894.
PR Checklist:
npm testlocally and all tests are passing.PR Description
It drops the port suffix in the
Hostheader for standard protocol + port combinations (HTTP +:80and HTTPS +:443). It partially reverts ff6d6c6, while still working for IPv6 addresses.CC: @JamesMGreene, @simov