Skip to content

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 <ilkka.myller@nodefield.com>
@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 <jasnell@gmail.com>
@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 <jasnell@gmail.com>
@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 <jasnell@gmail.com>
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 <jasnell@gmail.com>
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 <jasnell@gmail.com>
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 <jasnell@gmail.com>
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 <jasnell@gmail.com>
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 <jasnell@gmail.com>
@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