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

url: fix inconsistent port in url.resolveObject #8214

Closed
wants to merge 1 commit into from

Conversation

imyller
Copy link
Member

@imyller imyller commented Aug 21, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

url

Description of change

This commit fixes a bug where url.resolveObject returns conflicting host and port values.

$ node
> url.resolveObject('http://example.com:81', 'http://example.com:82');
Url {
  protocol: 'http:',
  slashes: true,
  auth: null,
  host: 'example.com:82',
  port: '81',
  hostname: 'example.com',
  hash: null,
  search: null,
  query: null,
  pathname: '/',
  path: '/',
  href: 'http://example.com:82/' }
> 

Fixes: #8213

This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: nodejs#8213

Signed-off-by: Ilkka Myller <[email protected]>
@nodejs-github-bot nodejs-github-bot added the url Issues and PRs related to the legacy built-in url module. label Aug 21, 2016
@jasnell
Copy link
Member

jasnell commented Aug 22, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/3792/
LGTM if CI is green.

@jasnell
Copy link
Member

jasnell commented Aug 23, 2016

New CI: https://ci.nodejs.org/job/node-test-pull-request/3806/
Red in last run is most likely unrelated but just in case...

jasnell pushed a commit that referenced this pull request Aug 23, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: #8213
PR-URL: #8214
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Aug 23, 2016

Landed in 286d44e . Thank you!

@jasnell jasnell closed this Aug 23, 2016
evanlucas pushed a commit that referenced this pull request Aug 24, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: #8213
PR-URL: #8214
Reviewed-By: James M Snell <[email protected]>
@MylesBorins
Copy link
Contributor

this does not land cleanly on v4.x @imyller would you be willing to manually backport against v4.x-staging

feel free to let me know if you have any questions

imyller added a commit to imyller/node that referenced this pull request Sep 30, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: nodejs#8213
PR-URL: nodejs#8214
Reviewed-By: James M Snell <[email protected]>
imyller added a commit to imyller/node that referenced this pull request Sep 30, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: nodejs#8213
PR-URL: nodejs#8214
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: #8213
Ref: #8872
PR-URL: #8214
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: #8213
Ref: #8872
PR-URL: #8214
Reviewed-By: James M Snell <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: #8213
Ref: #8872
PR-URL: #8214
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
This commit fixes bug where url.resolveObject returns conflicting
host and port values.

Fixes: #8213
Ref: #8872
PR-URL: #8214
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

url.resolveObject: conflicting values of host and port
4 participants