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

Connection leak in OpenShiftOAuthInterceptor #741

Closed
snjeza opened this issue Apr 23, 2017 · 1 comment
Closed

Connection leak in OpenShiftOAuthInterceptor #741

snjeza opened this issue Apr 23, 2017 · 1 comment

Comments

@snjeza
Copy link

snjeza commented Apr 23, 2017

See https://github.com/fabric8io/kubernetes-client/blob/master/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java#L85

"response.body().close();" releases a connection properly when using the standard http services
When using a web socket, "response.body().close();" doesn't release a connection.

The following OpenShiftOAuthInterceptor.intercept method fixes the issue:

@Override
    public Response intercept(Chain chain) throws IOException {
        Request request = chain.request();
        //Build new request
        Request.Builder builder = request.newBuilder();
        String token = oauthToken.get();
        if (Utils.isNotNullOrEmpty(token)) {
            setAuthHeader(builder, token);
        } else {
            if (Utils.isNotNullOrEmpty(config.getUsername()) && Utils.isNotNullOrEmpty(config.getPassword())) {
                synchronized (client) {
                  token = authorize();
                  if (token != null) {
                    oauthToken.set(token);
                  }
                }
              } else if (Utils.isNotNullOrEmpty(config.getOauthToken())) {
                token = config.getOauthToken();
                oauthToken.set(token);
              }
            if (Utils.isNotNullOrEmpty(token)) {
                setAuthHeader(builder, token);
            }
        }
        request = builder.build();
        return chain.proceed(request);
    }

The method adds an authorization token to all requests no matter if they require authentication or not.
Not sure if that is acceptable.

See https://issues.jboss.org/browse/CHE-180 for more detail.

@stale
Copy link

stale bot commented Aug 6, 2019

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale stale bot added the status/stale label Aug 6, 2019
@stale stale bot closed this as completed Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant