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

block all unsafe ports #5380

Merged
merged 3 commits into from
Oct 10, 2022
Merged

block all unsafe ports #5380

merged 3 commits into from
Oct 10, 2022

Conversation

kasperk81
Copy link
Contributor

extracted from three browsers' sources: dotnet/runtime#76236 (comment).

fix: #5379

@kasperk81
block all unsafe ports
756344d 
extracted from three browsers' sources: dotnet/runtime#76236 (comment).
@kasperk81 kasperk81 requested a review from a team as a code owner October 6, 2022 20:40
vlada-shubina
vlada-shubina reviewed Oct 7, 2022
View reviewed changes
Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

In general, I'm ok with the change, not sure if we need to specify the ports below 1024 though.

It would be good to add the content of the dotnet/runtime#76236 (comment) to code as the remark to know the source of information.

...osoft.TemplateEngine.Orchestrator.RunnableProjects/Macros/Config/GeneratePortNumberConfig.cs Outdated Show resolved Hide resolved
@JanKrivanek
Copy link
Member

In general, I'm ok with the change, not sure if we need to specify the ports below 1024 though.

It would be good to add the content of the dotnet/runtime#76236 (comment) to code as the remark to know the source of information.

The lower port bound is guarded to be >= 1024

templating/src/Microsoft.TemplateEngine.Orchestrator.RunnableProjects/Macros/GeneratePortNumberMacro.cs

Lines 50 to 53 in f9dc2bb

if (low < LowPortDefault)
{
low = LowPortDefault;
}

JanKrivanek
JanKrivanek approved these changes Oct 7, 2022
View reviewed changes
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

It would be nice to have the ports sorted - for easier human-readability of the code.
Other than that I'm fine with the change

vlada-shubina
vlada-shubina approved these changes Oct 7, 2022
View reviewed changes
Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

Thank you

@vlada-shubina vlada-shubina merged commit e0d5ffb into dotnet:main Oct 10, 2022
@JanKrivanek
Copy link
Member

Thank you @kasperk81!

vlada-shubina added a commit to vlada-shubina/templating that referenced this pull request Oct 10, 2022
@vlada-shubina
updated ports as in dotnet#5380
afaaf94
@vlada-shubina
Copy link
Member

/backport to release/7.0.2xx

@github-actions
Copy link
Contributor

Started backporting to release/7.0.2xx: https://github.com/dotnet/templating/actions/runs/3234311034

@github-actions github-actions bot mentioned this pull request Oct 12, 2022
Merged
vlada-shubina added a commit to vlada-shubina/templating that referenced this pull request Nov 4, 2022
@vlada-shubina
updated ports as in dotnet#5380
3b08298
vlada-shubina added a commit to vlada-shubina/templating that referenced this pull request Nov 9, 2022
@vlada-shubina
updated ports as in dotnet#5380
eb3b644
vlada-shubina added a commit that referenced this pull request Nov 9, 2022
@vlada-shubina
updated ports as in #5380
53ce47c
dotnet-bot pushed a commit to dotnet/dotnet that referenced this pull request Dec 6, 2022
@dotnet-maestro
[templating] Sync fedc320 → 4e62530
8e5a3c8 
Diff: https://github.com/dotnet/templating/compare/fedc32080e326a6ede705a7c1c64f8c3178cb84c..4e62530745f1c53b7796a35fbda531007cb62ece

From: dotnet/templating@fedc320
To: dotnet/templating@4e62530

Commits:
  - Update backport action environment (#5576)
    dotnet/templating@4b02183
  - Use backport workflow from arcade repo (#5586)
    dotnet/templating@9c4bbb6
  - made `separator` in join macro optional
    dotnet/templating@aad8d76
  - allowed other value types in constant macro config (and other configs)
    dotnet/templating@2c6607a
  - improved logging in `GeneratePortNumberMacro`
    dotnet/templating@915f4f4
  - improved `CoalesceMacro` logic
    dotnet/templating@405df47
  - macros 2.0
    dotnet/templating@b1cd547
  - docs
    dotnet/templating@7692b0f
  - localizations
    dotnet/templating@eeb7f1c
  - added better error handling
    dotnet/templating@411c119
  - Update docs/contributing/how-to-create-new-generated-symbol.md
    dotnet/templating@b75852f
  - removed IGeneratedSymbolMacro<T>
    dotnet/templating@8d6f53f
  - implemented deterministic mode
    dotnet/templating@5b01d3c
  - port refactoring after review
    dotnet/templating@1a7cc02
  - addressed review comments
    dotnet/templating@3d24c4a
  - updated docs - added information about which fields are mandatory
    dotnet/templating@70629dc
  - small fixes after self review
    dotnet/templating@8c70632
  - fixed `coalesce` macro behavior
    dotnet/templating@b8de40a
  - tests
    dotnet/templating@f17793a
  - updated ports as in dotnet/templating#5380
    dotnet/templating@53ce47c
  [... commit list trimmed ...]

[[ commit created by automation ]]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vlada-shubina vlada-shubina vlada-shubina approved these changes

@JanKrivanek JanKrivanek JanKrivanek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Applicationurl shouldn't randomly generate port 5060 or 5061
3 participants
@kasperk81 @JanKrivanek @vlada-shubina