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

google-oauth-java-client: AbstractAuthorizationCodeCallbackServlet is blocked because of locks #1071

Open
krog78 opened this issue Sep 25, 2023 · 0 comments

Comments

@krog78
Copy link

krog78 commented Sep 25, 2023

Environment details

https://github.com/googleapis/google-oauth-java-client/blob/main/google-oauth-client-servlet/src/main/java/com/google/api/client/extensions/servlet/auth/oauth2/AbstractAuthorizationCodeCallbackServlet.java

Steps to reproduce

  1. Send multiple times by second request to the callback URL
  2. Threads are blocked because of the ReentrantLock()

Code example

/** Lock on the flow. */
  private final Lock lock = new ReentrantLock();

  /**
   * Authorization code flow to be used across all HTTP servlet requests or {@code null} before
   * initialized in {@link #initializeFlow()}.
   */
  private AuthorizationCodeFlow flow;

  @Override
  protected final void doGet(HttpServletRequest req, HttpServletResponse resp)
      throws ServletException, IOException {
    StringBuffer buf = req.getRequestURL();
    if (req.getQueryString() != null) {
      buf.append('?').append(req.getQueryString());
    }
    AuthorizationCodeResponseUrl responseUrl = new AuthorizationCodeResponseUrl(buf.toString());
    String code = responseUrl.getCode();
    if (responseUrl.getError() != null) {
      onError(req, resp, responseUrl);
    } else if (code == null) {
      resp.setStatus(HttpServletResponse.SC_BAD_REQUEST);
      resp.getWriter().print("Missing authorization code");
    } else {
      lock.lock();
      try {
        if (flow == null) {
          flow = initializeFlow();
        }
        String redirectUri = getRedirectUri(req);
        TokenResponse response = flow.newTokenRequest(code).setRedirectUri(redirectUri).execute();
        String userId = getUserId(req);
        Credential credential = flow.createAndStoreCredential(response, userId);
        onSuccess(req, resp, credential);
      } finally {
        lock.unlock();
      }
    }
  }

Any additional information below

If there are lots of requests, the Threads are blocked and waiting for the lock from other threads.
What the purpose of the lock? Is it onlyuseful for initializeFlow()?
Is the rest of the process thread safe?

Can we put

 String redirectUri = getRedirectUri(req);
        TokenResponse response = flow.newTokenRequest(code).setRedirectUri(redirectUri).execute();
        String userId = getUserId(req);
        Credential credential = flow.createAndStoreCredential(response, userId);
        onSuccess(req, resp, credential);

outside the lock?

Thanks
Regards,
Sylvain

@krog78 krog78 changed the title google-oauth-java-client: AbstractCallBackServlet is blocked because of locks google-oauth-java-client: AbstractAuthorizationCodeCallbackServletis blocked because of locks Sep 25, 2023
@krog78 krog78 changed the title google-oauth-java-client: AbstractAuthorizationCodeCallbackServletis blocked because of locks google-oauth-java-client: AbstractAuthorizationCodeCallbackServlet is blocked because of locks Sep 25, 2023
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

No branches or pull requests

1 participant