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

proposal: x/tools/present: support being reverse proxied from https #21921

Closed
ddatsh opened this issue Sep 18, 2017 · 16 comments
Closed

proposal: x/tools/present: support being reverse proxied from https #21921

ddatsh opened this issue Sep 18, 2017 · 16 comments

Comments

@ddatsh
Copy link
Contributor

@ddatsh ddatsh commented Sep 18, 2017

Proposal

What did you do?

found
src/golang.org/x/tools/cmd/present/local.go:62
origin := &url.URL{Scheme: “http”}
fixed, not support https

src/golang.org/x/tools/godoc/static/static.go:2347
var websocket = new WebSocket(‘ws://’ + window.location.host + ‘/socket’);
fixed ws://,not support https

What did you expect to see?

support https access

@gopherbot gopherbot added this to the Unreleased milestone Sep 18, 2017
@ddatsh ddatsh changed the title x/tools/present:support https x/tools/present: support https Sep 18, 2017
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Sep 18, 2017

Why do you want this?

Do you want this for using present locally on your computer with https, or for using it remotely over the internet via https?

Also, are you aware of https://talks.godoc.org/? If not, does that help you achieve what you want to do?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 18, 2017

CC @campoy

@ddatsh
Copy link
Contributor Author

@ddatsh ddatsh commented Sep 19, 2017

@shurcooL want using present locally with https

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Sep 19, 2017

Since using HTTPS requires a TLS certificate (it could be self-signed, but that would display a red warning message that "the authenticity of this page cannot be verified", etc.), are you planning on bringing your own certificate to use with present, or is this issue also about finding a way to come up/create a certificate?

Doing the above seems pretty involved, and I'm not sure what the benefits would be. If you bind using the 127.0.0.0 address, connecting to it should be secure (as far as I know), since the connection uses your local network stack and cannot be seen by anyone external.

I'm not against this, I just want to understand the benefits and motivation better.

@ddatsh
Copy link
Contributor Author

@ddatsh ddatsh commented Sep 19, 2017

@shurcooL I just use reverse proxy to handle https,like caddy/nginx

https://present.xxx.com {
	gzip
	tls tls/present.xxx.com.crt tls/present.xxx.com.key {
		   alpn http/1.1
	}	
	proxy / http://127.0.0.1:3999

	proxy /socket 127.0.0.1:3999 {
		websocket
	}
}

but present play.js ,use fixed ws:// protocol

so ,if src/golang.org/x/tools/cmd/present/local.go
provide a new flag, support assign protocol, like

	https= flag.Bool("https", fale, "http/https")

origin := &url.URL{Scheme: “http or https”}

src/golang.org/x/tools/godoc/static/static.go
check location.protocol to new WebSocket(‘ws://’ or wss://

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Sep 19, 2017

I see.

Is your end goal to expose the present tool's output at an internet website (e.g., https://present.xxx.com in your sample Caddyfile)?

If so, I think it should absolutely support proxying from wss:// to ws:// protocol, does it not? /cc @mholt It should be very similar to reverse proxying external https:// traffic to your local backend via http:// (as far as I understand).

@ddatsh
Copy link
Contributor Author

@ddatsh ddatsh commented Sep 19, 2017

@shurcooL yes,expose the present to https website.
caddy or nginx support wss:// protocol, but present 's javascript, use fxed ws:// protocol

@ddatsh ddatsh changed the title x/tools/present: support https proposal: x/tools/present: support https Sep 19, 2017
@gopherbot gopherbot added the Proposal label Sep 19, 2017
@mholt
Copy link

@mholt mholt commented Sep 19, 2017

Just skimming this issue before I go to sleep, so forgive me if I've missed something; but I think you'll need to use wss:// to get a websocket connection on an HTTPS page.

@ddatsh
Copy link
Contributor Author

@ddatsh ddatsh commented Sep 19, 2017

@mholt you are right! so I think problem is present's javascript(src/golang.org/x/tools/godoc/static/playground.js) , should detetive location.protocol to new WebSocket using ws:// or wss:// ,

function SocketTransport() {
	var websocket = new WebSocket('ws://  or wss://' + window.location.host + '/socket');

and src/golang.org/x/tools/cmd/present/local.go, should supply a new flag to assign protocol(http or https)
origin := &url.URL{Scheme: "http or https"}

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Sep 19, 2017

You're right, the JavaScript will need to be updated to be able to connect to wss://, since that's what the reverse proxy will expose.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Sep 19, 2017

@ddatsh Would it be more accurate to rename this issue from "support https" to "support being reverse proxied from https" or so?

We don't need to add full fledged support for https (which comes with a lot of overhead and doesn't have meaningful benefits) to present tool to resolve your issue. We just need to add support for its output being proxied and accessible via https/wss. These would be much less invasive changes, and therefore much easier to accept.

@ddatsh ddatsh changed the title proposal: x/tools/present: support https proposal: x/tools/present: support being reverse proxied from https Sep 19, 2017
@rsc
Copy link
Contributor

@rsc rsc commented Sep 25, 2017

Yes, it seems like we should support having it behind a reverse proxy.

@ddatsh
Copy link
Contributor Author

@ddatsh ddatsh commented Feb 22, 2018

found can just modify caddyFile like

https://xx.xxx.com {
	gzip
	minify 
	tls tls/xxx.crt tls/xxx.key
	
	proxy / http://127.0.0.1:3999
	proxy /socket 127.0.0.1:3999 {
		header_upstream Origin http://127.0.0.1:3999
		header_upstream Host 127.0.0.1:3999
		websocket
	}
}

but still need modify godoc/static/playground.js

    var websocket;
    if(window.location.protocol == "http:"){
        websocket = new WebSocket('ws://' + window.location.host + '/socket');
    }else if(window.location.protocol == "https:"){
        websocket = new WebSocket('wss://' + window.location.host + '/socket');
    }
@ddatsh ddatsh closed this Feb 22, 2018
@ddatsh ddatsh reopened this Feb 22, 2018
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Feb 23, 2018

CL 96295 mentions this issue.

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 25, 2018

Change https://golang.org/cl/96295 mentions this issue: cmd/present: support being reverse proxied from https

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Feb 26, 2018

Thank you @ddatsh for addressing review comments and resolving this issue!

@golang golang locked and limited conversation to collaborators Feb 26, 2019
Bogdan-D pushed a commit to Bogdan-D/godocx that referenced this issue Nov 13, 2019
Fixes golang/go#21921

Change-Id: Idc6ff2773dcef7210ce4d0eecb7affb357a213b5
GitHub-Last-Rev: 50b959e9b6f2f1513235c5ca0124d80b487c5296
GitHub-Pull-Request: golang/tools#26
Reviewed-on: https://go-review.googlesource.com/96295
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants
You can’t perform that action at this time.