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 websocket support #97

Merged
merged 1 commit into from
Jun 17, 2016
Merged

Add websocket support #97

merged 1 commit into from
Jun 17, 2016

Conversation

ecordell
Copy link
Contributor

Updates goproxy to the fork supports websockets

@jzelinskie
Copy link
Contributor

Have we tried to get any of this upstreamed into goproxy? What's the progress on that?

@ecordell
Copy link
Contributor Author

This is the reverse proxy PR: elazarl/goproxy#143

}

func (proxy *ProxyHttpServer) proxyWebsocket(ctx *ProxyCtx, dest io.ReadWriter, source io.ReadWriter) {
errChan := make(chan error, 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why two? If one fails, do we really need to wait for the other to fail before exiting this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's buffered to 2 just in case both goroutines error out before the <- (protecting from a panic). If there's a reason to believe that can't happen I can change this

@jzelinskie jzelinskie assigned jakedt and unassigned jzelinskie Jun 17, 2016
@jakedt
Copy link

jakedt commented Jun 17, 2016

LGTM!!!11

@ecordell ecordell merged commit 8a548e5 into master Jun 17, 2016
@mshaposhnik
Copy link

How did this works? AFAIK ws connection request doesn't supports any extra headers like "Authorization".

@jzelinskie jzelinskie deleted the goproxy-ws branch June 19, 2018 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants