Use a list of restricted ports instead of allowing ports > 1023. #908
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… may still be restricted
mueschm
reviewed
May 11, 2023
| validate: (value) => { | ||
| const port = Number.parseInt(value); | ||
| if (!this.isValidPort(port)) { | ||
| return 'Port must be 80, 443, or any port between 1024 and 66535.'; | ||
| if (!port || port <= 0 || port > 65535) { |
There was a problem hiding this comment.
We also need the restricted port check here I believe.
There was a problem hiding this comment.
We do this question in a loop https://github.com/architect-team/architect-cli/pull/908/files#diff-c803da602bbb1783595bc04de6a99e122c48269d8c8eb1c6df6397bf5360f91bR495 so technically it still works if you type a restricted port (and asks the question again), though better UX to specifically say "Yo this port is restricted" - will update that 👍
|
🎉 This PR is included in version 1.41.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to the issue here: https://gitlab.com/architect-io/architect-cli/-/issues/662 and an update of #895.
Restricting ports to 80, 443, and > 1023 wasn't quite doing what we wanted because there's still restricted ports above 1024 (like 6000, which TJ ran into when randomly choosing a port).
Instead of limiting ports based on range, instead use Chrome/FF's restricted port list and don't allow those specifically.