This repository has been archived by the owner. It is now read-only.

ISSUE #3480 add test for non-reversable auth escaping #3488

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

Test against 2 known-bad strings
Test against 100 random strings

ISSUE 3480 add test for non-reversable auth escaping
Test against 2 known-bad strings
Test against 100 random strings

Can one of the admins verify this patch?

Closing this. Speaking as a former advocate for fuzz-y unit tests, they do not mix well with unit tests.

Would the feedback be to remove the 100 random strings then? Or was this tested in some other way in another test?

I believe this cases are tested by test/simple/test-url.js, but I could be mistaken. Please let me know if I'm wrong and I can reopen -- I'm a little unclear of the reasoning behind this test.

My feedback for this PR would be that it should match the style of the other tests -- no IIFE wrapper, jshint headers, add the licensing info, and remove fuzz tests. Reduce the number of paths to passing the tests -- i.e., it shouldn't be necessary to test direct equality, decodeURIComponent equality, and unescaped equality if we know what the expected inputs and outputs are.

coolaj86 commented Dec 1, 2014

The reasoning was that I found url.parse would encode certain strings with special (but HTTP allowed) characters in a way that they could not be decoded using either decodeURIComponent or unescape, meaning that url.parse was not idempotent.

I don't remember the particulars, but it was an edge case for HTTP Basic passwords with special character sequences that no one would probably use in practice.

I reran the test and it passes now, so I'm happy to leave the issue closed. Thanks for the feedback.

richardlau pushed a commit to ibmruntimes/node that referenced this pull request May 17, 2016

2016-05-17, Version 6.2.0 (Stable)
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna
  Henningsen) [#6511](nodejs/node#6511)
- **child_process**: use /system/bin/sh on android (Ben Noordhuis)
  [#6745](nodejs/node#6745)
- **deps**:
  - upgrade npm to 3.8.9 (Rebecca Turner)
    [#6664](nodejs/node#6664)
  - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh)
    [#6572](nodejs/node#6572)
  - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    [#6796](nodejs/node#6796)
  - Intl: ICU 57 bump (Steven R. Loomis)
    [#6088](nodejs/node#6088)
- **repl**:
  - copying tabs shouldn't trigger completion (Eugene Obrezkov)
    [#5958](nodejs/node#5958)
  - exports `Recoverable` (Blake Embrey)
    [#3488](nodejs/node#3488)
- **src**: add O_NOATIME constant (Rich Trott)
  [#6492](nodejs/node#6492)
- **src,module**: add --preserve-symlinks command line flag (James M
  Snell) [#6537](nodejs/node#6537)
- **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen)
  [#6683](nodejs/node#6683)

richardlau pushed a commit to ibmruntimes/node that referenced this pull request May 18, 2016

2016-05-17, Version 6.2.0 (Stable)
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna
  Henningsen) [#6511](nodejs/node#6511)
- **child_process**: use /system/bin/sh on android (Ben Noordhuis)
  [#6745](nodejs/node#6745)
- **deps**:
  - upgrade npm to 3.8.9 (Rebecca Turner)
    [#6664](nodejs/node#6664)
  - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh)
    [#6572](nodejs/node#6572)
  - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    [#6796](nodejs/node#6796)
  - Intl: ICU 57 bump (Steven R. Loomis)
    [#6088](nodejs/node#6088)
- **repl**:
  - copying tabs shouldn't trigger completion (Eugene Obrezkov)
    [#5958](nodejs/node#5958)
  - exports `Recoverable` (Blake Embrey)
    [#3488](nodejs/node#3488)
- **src**: add O_NOATIME constant (Rich Trott)
  [#6492](nodejs/node#6492)
- **src,module**: add --preserve-symlinks command line flag (James M
  Snell) [#6537](nodejs/node#6537)
- **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen)
  [#6683](nodejs/node#6683)

As of this release the 6.X line now includes 64-bit binaries for Linux
on Power Systems running in big endian mode in addition to the existing
64-bit binaries for running in little endian mode.

PR-URL: nodejs/node#6810

timkuijsten pushed a commit to timkuijsten/node that referenced this pull request May 24, 2016

2016-05-17, Version 6.2.0 (Stable)
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna
  Henningsen) [#6511](nodejs/node#6511)
- **child_process**: use /system/bin/sh on android (Ben Noordhuis)
  [#6745](nodejs/node#6745)
- **deps**:
  - upgrade npm to 3.8.9 (Rebecca Turner)
    [#6664](nodejs/node#6664)
  - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh)
    [#6572](nodejs/node#6572)
  - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    [#6796](nodejs/node#6796)
  - Intl: ICU 57 bump (Steven R. Loomis)
    [#6088](nodejs/node#6088)
- **repl**:
  - copying tabs shouldn't trigger completion (Eugene Obrezkov)
    [#5958](nodejs/node#5958)
  - exports `Recoverable` (Blake Embrey)
    [#3488](nodejs/node#3488)
- **src**: add O_NOATIME constant (Rich Trott)
  [#6492](nodejs/node#6492)
- **src,module**: add --preserve-symlinks command line flag (James M
  Snell) [#6537](nodejs/node#6537)
- **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen)
  [#6683](nodejs/node#6683)

As of this release the 6.X line now includes 64-bit binaries for Linux
on Power Systems running in big endian mode in addition to the existing
64-bit binaries for running in little endian mode.

PR-URL: nodejs/node#6810
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.