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

Add support for port 443 #129

Closed
wants to merge 2 commits into from
Closed

Add support for port 443 #129

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 13, 2017

A modified version of @superhawk610's Pull Request #128 that automatically adds :443 if a https is used rather that a http.

@ghost
Copy link
Author

ghost commented Jul 13, 2017

@superhawk610 Does this look okay?

@superhawk610
Copy link
Contributor

superhawk610 commented Jul 13, 2017

I don't have access to a HTTPS server to test but the logic looks sound, assuming 443 is an anonymous port and behaves the same way as 80.

Edit: Confirmed command returns without port whether specifying port 443 or not
image

Copy link
Contributor

@superhawk610 superhawk610 left a comment

Choose a reason for hiding this comment

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

Regex is correct for HTTPS connections.

@ghost ghost mentioned this pull request Jul 13, 2017
@alallier
Copy link
Owner

I was thinking about this PR. It shouldn't be needed after thinking about it. When using reload in command line mode we only create a http server not https. So we will never have to account for port 443. Of course the user could always specify port 443 in either express or command line mode and the port will be assigned based on that. The case that this PR account for should never hit

@ghost
Copy link
Author

ghost commented Jul 20, 2017

Closing due to @alallier's comment

@ghost ghost closed this Jul 20, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants