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 serving via ssl #296

Closed
wants to merge 2 commits into from
Closed

Conversation

JakeDetels
Copy link
Contributor

Allows users to run server using SSL via broccoli serve -ssl.

SSL key and certificate files must be located in the user's project root at ssl/server.key and ssl/server.crt.

@JakeDetels JakeDetels changed the title Ssl add support for serving via ssl Feb 7, 2016
@@ -15,6 +15,7 @@ function broccoliCLI () {
.description('start a broccoli server')
.option('--port <port>', 'the port to bind to [4200]', 4200)
.option('--host <host>', 'the host to bind to [localhost]', 'localhost')
.option('--ssl', 'serve via https')
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets do:

  --ssl (Boolean) (Default: false)
  --ssl-key (String) (Default: ssl/server.key)
  --ssl-cert (String) (Default: ssl/server.crt)

var sslOptions
try {
sslOptions = {
key: fs.readFileSync(path.join(process.cwd(), 'ssl/server.key')),
Copy link
Contributor

Choose a reason for hiding this comment

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

path.join(process.cwd() here is not needed, cwd is implied when using fs.* methods

cert: fs.readFileSync(path.join(process.cwd(), 'ssl/server.crt'))
}
} catch(e) {
throw new Error('SSL key and certificate files must be located at ssl/server.key and ssl/server.crt')
Copy link
Contributor

Choose a reason for hiding this comment

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

this error message likely should include cwd in the errors.

@stefanpenner
Copy link
Contributor

@JakeDetels sorry for the late review, if you can make the above changes. We can merge and :shipit:

@JakeDetels
Copy link
Contributor Author

@stefanpenner: I'll update this by end-of-month..

@rwjblue
Copy link
Member

rwjblue commented Mar 19, 2018

Friendly ping 😺

@stefanpenner
Copy link
Contributor

landed via #417

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.

3 participants