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

test: test about:blank against invalid WHATWG URL #20796

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

If failure is true, parsing about:blank against base
must give failure. This tests that the logic for converting
base URLs into strings properly fails the whole parsing
algorithm if the base URL cannot be parsed.

Fixes: #20720

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

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label May 17, 2018
> If `failure` is true, parsing `about:blank` against `base`
> must give failure. This tests that the logic for converting
> base URLs into strings properly fails the whole parsing
> algorithm if the base URL cannot be parsed.

Fixes: nodejs#20720
@joyeecheung
Copy link
Member Author

@joyeecheung
Copy link
Member Author

Copy link
Member

@apapirovski apapirovski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 17, 2018
@apapirovski
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/15012/ (then it can land)

@joyeecheung
Copy link
Member Author

joyeecheung commented May 22, 2018

@apapirovski Thank you for starting the CI. The two failures on vs2017/win2016 seems irrelevant.

See errors
not ok 51 parallel/test-child-process-fork-closed-channel-segfault
  ---
  duration_ms: 0.263
  severity: fail
  exitcode: 1
  stack: |-
    c:\workspace\node-test-binary-windows\test\parallel\test-child-process-fork-closed-channel-segfault.js:70
                throw err;
                ^
    
    Error: write EMFILE
        at ChildProcess.target._send (internal/child_process.js:741:20)
        at ChildProcess.target.send (internal/child_process.js:625:19)
        at Worker.send (internal/cluster/worker.js:40:28)
        at Socket.<anonymous> (c:\workspace\node-test-binary-windows\test\parallel\test-child-process-fork-closed-channel-segfault.js:36:16)
        at Object.onceWrapper (events.js:273:13)
        at Socket.emit (events.js:182:13)
        at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1147:10)
  ...
not ok 52 parallel/test-child-process-fork-net
  ---
  duration_ms: 0.335
  severity: fail
  exitcode: 1
  stack: |-
    PARENT: server listening
    CHILD: server listening
    CLIENT: connected
    PARENT: got connection
    CLIENT: connected
    CHILD: got connection
    CLIENT: closed
    CHILD: got connection
    CHILD: got connection
    CLIENT: closed
    CLIENT: connected
    CLIENT: connected
    CLIENT: closed
    CLIENT: closed
    PARENT: server closed
    CHILD: got socket
    CLIENT: got data
    CLIENT: closed
    testSocket, listening
    events.js:167
          throw er; // Unhandled 'error' event
          ^
    
    Error: write EPIPE
        at ChildProcess.target._send (internal/child_process.js:741:20)
        at ChildProcess.target.send (internal/child_process.js:625:19)
        at SocketListSend._request (internal/socket_list.js:20:16)
        at SocketListSend.close (internal/socket_list.js:40:10)
        at Server.close (net.js:1624:24)
        at Socket.<anonymous> (c:\workspace\node-test-binary-windows\test\parallel\test-child-process-fork-net.js:179:16)
        at Socket.emit (events.js:182:13)
        at TCP._handle.close [as _onclose] (net.js:596:12)
    Emitted 'error' event at:
        at process.nextTick (internal/child_process.js:745:39)
        at process._tickCallback (internal/process/next_tick.js:61:11)
  ...

Landed in a5aad24

joyeecheung added a commit that referenced this pull request May 22, 2018
> If `failure` is true, parsing `about:blank` against `base`
> must give failure. This tests that the logic for converting
> base URLs into strings properly fails the whole parsing
> algorithm if the base URL cannot be parsed.

Fixes: #20720

PR-URL: #20796
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request May 22, 2018
> If `failure` is true, parsing `about:blank` against `base`
> must give failure. This tests that the logic for converting
> base URLs into strings properly fails the whole parsing
> algorithm if the base URL cannot be parsed.

Fixes: #20720

PR-URL: #20796
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@addaleax addaleax mentioned this pull request May 22, 2018
MylesBorins pushed a commit that referenced this pull request May 23, 2018
> If `failure` is true, parsing `about:blank` against `base`
> must give failure. This tests that the logic for converting
> base URLs into strings properly fails the whole parsing
> algorithm if the base URL cannot be parsed.

Fixes: #20720

PR-URL: #20796
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggested additional WHATWG URL test coverage
7 participants