Skip to content

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

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
Closed
anonrig wants to merge 2 commits into from
Closed

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

anonrig wants to merge 2 commits into from

Conversation

@anonrig
Copy link
Member

@anonriganonrig commented Jul 21, 2023•
edited
Loading

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
Copy link
Collaborator

nodejs-github-bot commented Jul 21, 2023

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
@anonriganonrig requested review from aduh95 and lpincaJuly 21, 2023 17:26
lpinca
lpinca approved these changes Jul 21, 2023
View reviewed changes
@anonriganonrigforce-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
Copy link
Contributor

github-actionsbot commented Jul 21, 2023

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

@anonriganonrig 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
Copy link
Collaborator

nodejs-github-bot commented Jul 21, 2023

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

aduh95
aduh95 approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@aduh95aduh95 left a comment

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
Copy link
MemberAuthor

anonrig commented Jul 21, 2023

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
Copy link
Collaborator

nodejs-github-bot commented Jul 21, 2023

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
Copy link
Collaborator

nodejs-github-bot commented Jul 22, 2023

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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
This was referenced Jul 28, 2023
Open
Open
Open
pluris pushed a commit to pluris/node that referenced this pull request Aug 6, 2023
@anonrig
test: validate host with commas on url.parse
f1d25bc
PR-URL: nodejs#48878 Refs: nodejs#48873 Refs: nodejs#48855 Refs: nodejs#48850 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
pluris pushed a commit to pluris/node that referenced this pull request Aug 6, 2023
@anonrig
lib: remove invalid parameter to toASCII
c40e40a
PR-URL: nodejs#48878 Refs: nodejs#48873 Refs: nodejs#48855 Refs: nodejs#48850 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
pluris pushed a commit to pluris/node that referenced this pull request Aug 7, 2023
@anonrig
test: validate host with commas on url.parse
24e9fb4
PR-URL: nodejs#48878 Refs: nodejs#48873 Refs: nodejs#48855 Refs: nodejs#48850 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
pluris pushed a commit to pluris/node that referenced this pull request Aug 7, 2023
@anonrig
lib: remove invalid parameter to toASCII
161ff90
PR-URL: nodejs#48878 Refs: nodejs#48873 Refs: nodejs#48855 Refs: nodejs#48850 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@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