Skip to content

test: validate host with commas on url.parse - #48878

Closed
anonrig wants to merge 2 commits into
nodejs:mainfrom
anonrig:validate-url-parse-commas
Closed

test: validate host with commas on url.parse#48878
anonrig wants to merge 2 commits into
nodejs:mainfrom
anonrig:validate-url-parse-commas

Conversation

@anonrig

@anonriganonrig commented Jul 21, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Add missing test for url's with commas for url.parse and removes unnecessary parameter to toASCII

Ref: #48873
Ref: #48855
Ref: #48850

cc @nodejs/url @aduh95

@nodejs-github-bot

nodejs-github-bot commented Jul 21, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/url

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. url Issues and PRs related to the legacy built-in url module. labels Jul 21, 2023
@anonriganonrig added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Jul 21, 2023
@anonrig
anonrig requested review from aduh95 and lpincaJuly 21, 2023 17:26
lpinca
lpinca approved these changes Jul 21, 2023
View reviewed changes
Comment threadtest/parallel/test-url-parse-format.js Outdated
@anonrig
anonrigforce-pushed the validate-url-parse-commas branch from c44e16f to faafb4fCompareJuly 21, 2023 18:36
@anonriganonrig added fast-track PRs that do not need to wait for 48 hours to land. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 21, 2023
@github-actions

github-actionsBot commented Jul 21, 2023

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @anonrig. Please 👍 to approve.

@anonrig
anonrig requested a review from lpincaJuly 21, 2023 18:38
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 21, 2023
@nodejs-github-bot

nodejs-github-bot commented Jul 21, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52894/

aduh95
aduh95 approved these changes Jul 21, 2023
View reviewed changes

@aduh95aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Is the test not related to the lib/ change, and that's why there are two commits?

@anonrig

anonrig commented Jul 21, 2023

Copy link
Copy Markdown
MemberAuthor

LGTM. Is the test not related to the lib/ change, and that's why there are two commits?

The test is not related to the other commit. We simply forgot to remove the unnecessary parameter there, and this bug lead to realize that.

@nodejs-github-bot

nodejs-github-bot commented Jul 21, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52898/

@github-actionsgithub-actionsBot mentioned this pull request Jul 22, 2023
Open
12 tasks
@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 22, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 22, 2023
@nodejs-github-bot

nodejs-github-bot commented Jul 22, 2023

Copy link
Copy Markdown
Collaborator

Landed in b68fa59...9dd574c

nodejs-github-bot pushed a commit that referenced this pull request Jul 22, 2023
@anonrig@nodejs-github-bot
test: validate host with commas on url.parse
f4863fa
PR-URL: #48878
Refs: #48873
Refs: #48855
Refs: #48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Jul 22, 2023
@anonrig@nodejs-github-bot
lib: remove invalid parameter to toASCII
9dd574c
PR-URL: #48878
Refs: #48873
Refs: #48855
Refs: #48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@anonriganonrig mentioned this pull request Jul 22, 2023
Closed
anonrig added a commit to anonrig/node that referenced this pull request Jul 22, 2023
@anonrig
test: validate host with commas on url.parse
a0edb33
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
anonrig added a commit to anonrig/node that referenced this pull request Jul 22, 2023
@anonrig
lib: remove invalid parameter to toASCII
60f1bb5
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This was referenced Jul 23, 2023
Open
Open
Open
Open
Open
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Jul 27, 2023
@anonrig@Ceres6
test: validate host with commas on url.parse
932d92e
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Jul 27, 2023
@anonrig@Ceres6
lib: remove invalid parameter to toASCII
9923dd9
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This was referenced Jul 28, 2023
Open
Open
Open
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@anonrig@Ceres6
test: validate host with commas on url.parse
ba96a46
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@anonrig@Ceres6
lib: remove invalid parameter to toASCII
c433de3
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@anonrig@Ceres6
test: validate host with commas on url.parse
6c0c565
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@anonrig@Ceres6
lib: remove invalid parameter to toASCII
bb7ffa8
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023
@anonrig@UlisesGascon
test: validate host with commas on url.parse
17a1be6
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023
@anonrig@UlisesGascon
lib: remove invalid parameter to toASCII
5affce3
PR-URL: nodejs#48878
Refs: nodejs#48873
Refs: nodejs#48855
Refs: nodejs#48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
@anonrig@RafaelGSS
test: validate host with commas on url.parse
3680a66
PR-URL: #48878
Refs: #48873
Refs: #48855
Refs: #48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
@anonrig@RafaelGSS
lib: remove invalid parameter to toASCII
420b85c
PR-URL: #48878
Refs: #48873
Refs: #48855
Refs: #48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@UlisesGasconUlisesGascon mentioned this pull request Aug 15, 2023
Merged
ruyadorno pushed a commit that referenced this pull request Aug 16, 2023
@anonrig@ruyadorno
test: validate host with commas on url.parse
2de9868
PR-URL: #48878
Backport-PR-URL: #48873
Refs: #48873
Refs: #48855
Refs: #48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
ruyadorno pushed a commit that referenced this pull request Aug 16, 2023
@anonrig@ruyadorno
lib: remove invalid parameter to toASCII
69aaf8b
PR-URL: #48878
Backport-PR-URL: #48873
Refs: #48873
Refs: #48855
Refs: #48850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@ruyadornoruyadorno mentioned this pull request Aug 17, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamingrbenjamingrbenjamingr approved these changes
@lpincalpincalpinca approved these changes
@aduh95aduh95aduh95 approved these changes

Assignees

No one assigned

Labels

commit-queue-rebaseAdd this label to allow the Commit Queue to land a PR in several commits.fast-trackPRs that do not need to wait for 48 hours to land.needs-ciPRs that need a full CI run.urlIssues and PRs related to the legacy built-in url module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@anonrig@nodejs-github-bot@benjamingr@lpinca@aduh95@ruyadorno