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.

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

Allow to specify the local endpoint for HttpListener #397

Closed
wants to merge 3 commits into from

Conversation

Alex-111
Copy link
Contributor

@Alex-111 Alex-111 commented Jul 27, 2023

Description

Currently the HttpListener does not allow to listen on a specific interface. It always is listening to the first interfase it finds. E.g. if you have more than one it always binds to the first interface.

Motivation and Context

The HttpListener should be able to bind e.g. to the second network interface...

@nfbot
Copy link
Member

nfbot commented Jul 27, 2023

Hi @Alex-111,

😯 I'm afraid you'll have to use the PR template like the rest of us...
Make sure you've used the template and have include all the required information and fill in the appropriate details. After doing that feel free to reopen the PR. If you have questions we are here to help..

@Alex-111
Copy link
Contributor Author

@dotnet-policy-service agree

@nfbot
Copy link
Member

nfbot commented Jul 27, 2023

Hi @Alex-111,

😯 I'm afraid you'll have to use the PR template like the rest of us...
Make sure you've used the template and have include all the required information and fill in the appropriate details. After doing that feel free to reopen the PR. If you have questions we are here to help..

@Ellerbach
Copy link
Member

@Alex-111 please use the PR template, there are automations based on it!

@josesimoes
Copy link
Member

@Alex-111 I've just merged upstream changes with code style fixes so the changes on the PR are only related with the matter.

@josesimoes josesimoes changed the title allow to specify the local endpoint for HttpListener Allow to specify the local endpoint for HttpListener Jul 27, 2023
@nfbot
Copy link
Member

nfbot commented Jul 27, 2023

Hi @Alex-111,

😯 I'm afraid you'll have to use the PR template like the rest of us...
Make sure you've used the template and have include all the required information and fill in the appropriate details. After doing that feel free to reopen the PR. If you have questions we are here to help..

@Alex-111 Alex-111 closed this Jul 27, 2023
@nfbot nfbot added the invalid This doesn't seem right label Jul 27, 2023
@josesimoes
Copy link
Member

@Alex-111 please don't give up on the PR! Nor get spooked with the messages from nfbot. 😄
It's a matter of having back the PR comment template and it's ready for review.

@josesimoes
Copy link
Member

@Alex-111 never mind! Just saw that you've started over. Thanks!

@Alex-111
Copy link
Contributor Author

Alex-111 commented Jul 27, 2023

@josesimoes
I'm not sure how much the core .net classes like HttpListener has to be aligned with the full framework. In full framework the above feature seems to be available via Prefixes, but at the moment there is no such an API in nanoFramework available...
Are there any best practices for nanoframework how new features should be implemented?
is it allowed to have a different API in nanoframework than in full framework?

@josesimoes
Copy link
Member

@Alex-111 the strategy is to have the exact same API as the full framework. When that is not possible/doesn't make sense we're OK to deviate. This seams to be the case.
I'm OK with your proposed change. It's not breaking anything and it's implemented in a reasonable way. Even for discoverability.

@Ellerbach would you agree?

@Alex-111
Copy link
Contributor Author

Well. Full Framework would look like this:

HttpListener listener = new HttpListener(); listener.Prefixes.Add("http://10.0.0.1:80/"); listener.Start();

i.e. there shouldn't be a "port" and also no "localEndpointIP" parameter at all. But as we already have port I would agree to leave it as it is now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants