Skip to content

Conversation

@ericprud
Copy link
Contributor

redirectFrom: 80, or
redirectFrom: [80 8088 8888]

 ISSUES: pretty logging: currently monoton: solid:settings Server URI: https://ericp.localhost:8443 +0ms ... solid:settings SSL Certificate path: ../self/keys/localhost2.cert +0ms 1-> will redirect 8080 to 8443 Solid server (v4.0.1-9-g3e44601) running on https://localhost:8443/ Press <ctrl>+c to stop solid:authentication Provider keys loaded from config +13ms ... solid:authentication Local RP client initialized +0ms 2-> localhost => https://localhost:8443/foo/bar 2-> localhost => https://localhost:8443/foo/bar 

1 appears once per entry in the array of redirectFrom ports.
2 appears once per executed redirect.

 redirectFrom: 80, or redirectFrom: [80 8088 8888]
Copy link
Contributor

@melvincarvalhomelvincarvalho left a comment

Choose a reason for hiding this comment

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

Looks good. Couple of things :

  1. Why would you want to redirect from multiple ports?

  2. Any testing of this?

console.log(host, '=> https://' + host + portStr + req.url)
res.redirect('https://' + host + portStr + req.url)
})
redirectingServer.listen(redirectFrom);
Copy link
Contributor

Choose a reason for hiding this comment

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

no semi colon here

@melvincarvalho
Copy link
Contributor

Copy link
Contributor

@timbltimbl left a comment

Choose a reason for hiding this comment

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

Could you check that everything including this works just the same in command line and in config file? That is the policy we decided on last summer IIRC.

@timbl
Copy link
Contributor

timbl commented Feb 7, 2018

Needs standard processing to pass tests?

@ericprudericprud merged commit e87a8aa into masterFeb 9, 2018
@ericprudericprud deleted the redirect_http_to_https branch February 9, 2018 15:24
@melvincarvalho
Copy link
Contributor

Hi @ericprud a couple of things

  1. Would it be OK to associate PR's with issues? (for next time)
  2. Should be bump the minor version of node-solid-server?
npm version patch -m "Release version %s of the npm package." git push git push --tags npm publish 

I think should do it

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ericprud@melvincarvalho@timbl