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

http serve: pass the hostname as is to user-defined onListen handler #24776

Closed
kt3k opened this issue Jul 29, 2024 · 0 comments · Fixed by #24777
Closed

http serve: pass the hostname as is to user-defined onListen handler #24776

kt3k opened this issue Jul 29, 2024 · 0 comments · Fixed by #24777
Labels
feat new feature (which has been agreed to/accepted)

Comments

@kt3k
Copy link
Member

kt3k commented Jul 29, 2024

Currently we substitute 'localhost' to hostname if the platform is windows and the hostname is "0.0.0.0" to work around the issue of browsers in windows.

deno/ext/http/00_serve.ts

Line 664 in ad5cec2

addr.hostname = hostname;

This makes sense for displaying something more convenient for users, but I think it's not appropriate if the user provide onListen callback to serve because the user might want to do more than just displaying it using the hostname string (One such idea is denoland/std#5558 ).

I suggest we should move that 'localhost'-substitution logic inside the default onListen implementation, instead of the current place.
@kt3k kt3k mentioned this issue Jul 29, 2024
Merged
@lucacasonato lucacasonato added the feat new feature (which has been agreed to/accepted) label Jul 31, 2024
kt3k added a commit that referenced this issue Sep 5, 2024
@kt3k
fix(ext/http): do not set localhost to hostname unnecessarily (#24777)
e799c28 
This commit changes when to cause the hostname substition of `0.0.0.0` ->
`localhost`.

Currently we substitute `localhost` to the hostname on windows before
calling `options.onListen`, which prevents the users to do more advanced
thing using hostname string like
denoland/std#5558. This PR changes it not to
substitute it when the user provide `onListen` callback.

closes #24776
unblocks denoland/std#5558
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feat new feature (which has been agreed to/accepted)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ext/http): do not set localhost to hostname unnecessarily kt3k/deno
2 participants
@kt3k @lucacasonato