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.

Sign up for GitHub

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

Jump to bottom

URL empty passwords #10595

Closed
stevenvachon opened this issue Jan 3, 2017 · 5 comments
Closed

URL empty passwords #10595

stevenvachon opened this issue Jan 3, 2017 · 5 comments
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@stevenvachon
Copy link

The spec was updated, so we should do so as well.
@jasnell
Copy link
Member

jasnell commented Jan 3, 2017

To be clear, we already support the case where the password is blank (e.g. http://foo:@example.org and http://:@example.org). Where we currently differ is when the : is omitted (e.g. http://foo@example.org and http://@example.org).

@mscdex mscdex added the url Issues and PRs related to the legacy built-in url module. label Jan 3, 2017
@jasnell
Copy link
Member

jasnell commented Jan 3, 2017

Actually, I take that back. The password getter returns '' appropriately. The inspect code does not show the empty password, which I think is what we actually want (to show the actual values if they exist).

new url.URL('http://@example.org'); // password === ''
new url.URL('http://:@example.,org'); // password === ''
new url.URL('http://foo@example.org'); // password === ''

@domenic
Copy link
Contributor

domenic commented Jan 3, 2017

The main thing you'll want to do is to pass the newly updated tests: web-platform-tests/wpt@e001240

jasnell added a commit to jasnell/node that referenced this issue Jan 4, 2017
@jasnell
url: change null password handling
7588628 
Pulls in new URL parsing tests from w3c web-platform-tests and updates
null password handling.

Refs: web-platform-tests/wpt@e001240
Refs: whatwg/url#186.
Fixes: nodejs#10595
@jasnell jasnell mentioned this issue Jan 4, 2017
Closed
3 tasks
@joyeecheung joyeecheung added the whatwg-url Issues and PRs related to the WHATWG URL implementation. label Jan 5, 2017
@mscdex mscdex removed the url Issues and PRs related to the legacy built-in url module. label Jan 5, 2017
@jasnell jasnell closed this as completed in 5161b00 Jan 6, 2017
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 18, 2017
@jasnell @italoacasas
url: change null password handling
e858f6c 
Pulls in new URL parsing tests from w3c web-platform-tests and updates
null password handling.

Refs: web-platform-tests/wpt@e001240
Refs: whatwg/url#186
PR-URL: nodejs#10601
Fixes: nodejs#10595
Reviewed-By: Michal Zasso <targos@protonmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 19, 2017
@jasnell @italoacasas
url: change null password handling
7c77852 
Pulls in new URL parsing tests from w3c web-platform-tests and updates
null password handling.

Refs: web-platform-tests/wpt@e001240
Refs: whatwg/url#186
PR-URL: nodejs#10601
Fixes: nodejs#10595
Reviewed-By: Michal Zasso <targos@protonmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 24, 2017
@jasnell @italoacasas
url: change null password handling
2f0a1c8 
Pulls in new URL parsing tests from w3c web-platform-tests and updates
null password handling.

Refs: web-platform-tests/wpt@e001240
Refs: whatwg/url#186
PR-URL: nodejs#10601
Fixes: nodejs#10595
Reviewed-By: Michal Zasso <targos@protonmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 27, 2017
@jasnell @italoacasas
url: change null password handling
e46bdcf 
Pulls in new URL parsing tests from w3c web-platform-tests and updates
null password handling.

Refs: web-platform-tests/wpt@e001240
Refs: whatwg/url#186
PR-URL: nodejs#10601
Fixes: nodejs#10595
Reviewed-By: Michal Zasso <targos@protonmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@stevenvachon
Copy link
Author

Can we get this released, please?

@TimothyGu
Copy link
Member

@stevenvachon, it is on the agenda: #11062

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mscdex @stevenvachon @jasnell @domenic @TimothyGu @joyeecheung