Skip to content

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Feb 23, 2016

Description of change

This fixes an off-by-one error with url.parse(). Ref #5393.

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

  • url

@mscdex mscdex added the url Issues and PRs related to the legacy built-in url module. label Feb 23, 2016
@MylesBorins
Copy link
Contributor

LGTM if CI is good (guess we're waiting a minue)

@mscdex
Copy link
Contributor Author

mscdex commented Feb 23, 2016

I'm not sure when CI will be back up? FWIW make test passes for me locally.

@silverwind
Copy link
Contributor

LGTM, tests are passing locally for me as well.

@rvagg
Copy link
Member

rvagg commented Feb 24, 2016

lgtm

@evanlucas
Copy link
Contributor

LGTM

@Fishrock123
Copy link
Contributor

linking to release proposal: #5400

@mscdex
Copy link
Contributor Author

mscdex commented Feb 25, 2016

@mscdex
Copy link
Contributor Author

mscdex commented Feb 25, 2016

CI is all green.

@Fishrock123
Copy link
Contributor

@mscdex is this good to land?

@mscdex
Copy link
Contributor Author

mscdex commented Feb 26, 2016

@Fishrock123 As far as I can tell. It passed CI.

@rvagg rvagg mentioned this pull request Feb 27, 2016
5 tasks
silverwind pushed a commit that referenced this pull request Feb 27, 2016
Fixes: #5393
PR-URL: #5394
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@silverwind
Copy link
Contributor

Landed in 8b16ba3.

@silverwind silverwind closed this Feb 27, 2016
@silverwind silverwind added this to the 5.7.1 milestone Feb 27, 2016
rvagg pushed a commit that referenced this pull request Feb 28, 2016
Fixes: #5393
PR-URL: #5394
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@jasnell
Copy link
Member

jasnell commented Mar 2, 2016

Marking as don't land on v4... I don't believe the change that led to this regression was ever backported to v4 (/cc @thealphanerd )... please change that if necessary, however.

Fishrock123 added a commit that referenced this pull request Mar 2, 2016
Notable changes:

* governance: The Core Technical Committee (CTC) added four new members
to help guide Node.js core development: Evan Lucas, Rich Trott, Ali
Ijaz Sheikh and Сковорода Никита Андреевич (Nikita Skovoroda).

* openssl: Upgrade from 1.0.2f to 1.0.2g (Ben Noordhuis)
#5507
  - Fix a double-free defect in parsing malformed DSA keys that may
potentially be used for DoS or memory corruption attacks. It is likely
to be very difficult to use this defect for a practical attack and is
therefore considered low severity for Node.js users. More info is
available at https://www.openssl.org/news/vulnerabilities.html#2016-0705
  - Fix a defect that can cause memory corruption in certain very rare
cases relating to the internal `BN_hex2bn()` and `BN_dec2bn()`
functions. It is believed that Node.js is not invoking the code paths
that use these functions so practical attacks via Node.js using this
defect are _unlikely_ to be possible. More info is available at
https://www.openssl.org/news/vulnerabilities.html#2016-0797
  - Fix a defect that makes the CacheBleed Attack
(https://ssrg.nicta.com.au/projects/TS/cachebleed/) possible. This
defect enables attackers to execute side-channel attacks leading to the
potential recovery of entire RSA private keys. It only affects the
Intel Sandy Bridge (and possibly older) microarchitecture when using
hyper-threading. Newer microarchitectures, including Haswell, are
unaffected. More info is available at
https://www.openssl.org/news/vulnerabilities.html#2016-0702

* Fixed several regressions that appeared in v5.7.0:
  - path.relative():
    - Output is no longer unnecessarily verbose (Brian White)
#5389
    - Resolving UNC paths on Windows now works correctly (Owen Smith)
#5456
    - Resolving paths with prefixes now works correctly from the root
directory (Owen Smith) #5490
  - url: Fixed an off-by-one error with `parse()` (Brian White)
#5394
  - dgram: Now correctly handles a default address case when offset and
length are specified (Matteo Collina)
#5407

PR-URL: #5464
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Mar 2, 2016
Notable changes:

* governance: The Core Technical Committee (CTC) added four new members
to help guide Node.js core development: Evan Lucas, Rich Trott, Ali
Ijaz Sheikh and Сковорода Никита Андреевич (Nikita Skovoroda).

* openssl: Upgrade from 1.0.2f to 1.0.2g (Ben Noordhuis)
nodejs#5507
  - Fix a double-free defect in parsing malformed DSA keys that may
potentially be used for DoS or memory corruption attacks. It is likely
to be very difficult to use this defect for a practical attack and is
therefore considered low severity for Node.js users. More info is
available at https://www.openssl.org/news/vulnerabilities.html#2016-0705
  - Fix a defect that can cause memory corruption in certain very rare
cases relating to the internal `BN_hex2bn()` and `BN_dec2bn()`
functions. It is believed that Node.js is not invoking the code paths
that use these functions so practical attacks via Node.js using this
defect are _unlikely_ to be possible. More info is available at
https://www.openssl.org/news/vulnerabilities.html#2016-0797
  - Fix a defect that makes the CacheBleed Attack
(https://ssrg.nicta.com.au/projects/TS/cachebleed/) possible. This
defect enables attackers to execute side-channel attacks leading to the
potential recovery of entire RSA private keys. It only affects the
Intel Sandy Bridge (and possibly older) microarchitecture when using
hyper-threading. Newer microarchitectures, including Haswell, are
unaffected. More info is available at
https://www.openssl.org/news/vulnerabilities.html#2016-0702

* Fixed several regressions that appeared in v5.7.0:
  - path.relative():
    - Output is no longer unnecessarily verbose (Brian White)
nodejs#5389
    - Resolving UNC paths on Windows now works correctly (Owen Smith)
nodejs#5456
    - Resolving paths with prefixes now works correctly from the root
directory (Owen Smith) nodejs#5490
  - url: Fixed an off-by-one error with `parse()` (Brian White)
nodejs#5394
  - dgram: Now correctly handles a default address case when offset and
length are specified (Matteo Collina)
nodejs#5407

PR-URL: nodejs#5464
@mscdex mscdex deleted the url-fix-parse-off-by-one branch March 4, 2016 23:40
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.

7 participants