Skip to content

Commit b51f37f

Browse files
committed
Merge pull request #128 from gcurtis/oauth-race
Fix OAuth race condition when refreshing
2 parents 19875bc + 985dcfd commit b51f37f

File tree

3 files changed

+56
-16
lines changed

3 files changed

+56
-16
lines changed

src/main/java/com/box/sdk/BoxAPIConnection.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,4 +507,26 @@ public String save() {
507507
.add("maxRequestAttempts", this.maxRequestAttempts);
508508
return state.toString();
509509
}
510+
511+
String lockAccessToken() {
512+
if (this.autoRefresh && this.canRefresh() && this.needsRefresh()) {
513+
this.refreshLock.writeLock().lock();
514+
try {
515+
if (this.needsRefresh()) {
516+
this.refresh();
517+
}
518+
this.refreshLock.readLock().lock();
519+
} finally {
520+
this.refreshLock.writeLock().unlock();
521+
}
522+
} else {
523+
this.refreshLock.readLock().lock();
524+
}
525+
526+
return this.accessToken;
527+
}
528+
529+
void unlockAccessToken() {
530+
this.refreshLock.readLock().unlock();
531+
}
510532
}

src/main/java/com/box/sdk/BoxAPIRequest.java

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ private BoxAPIResponse trySend(ProgressListener listener) {
347347
}
348348

349349
if (this.api != null) {
350-
connection.addRequestProperty("Authorization", "Bearer " + this.api.getAccessToken());
350+
connection.addRequestProperty("Authorization", "Bearer " + this.api.lockAccessToken());
351351
connection.setRequestProperty("User-Agent", this.api.getUserAgent());
352352

353353
if (this.api instanceof SharedLinkAPIConnection) {
@@ -363,26 +363,34 @@ private BoxAPIResponse trySend(ProgressListener listener) {
363363
}
364364

365365
this.requestProperties = connection.getRequestProperties();
366-
this.writeBody(connection, listener);
367366

368-
// Ensure that we're connected in case writeBody() didn't write anything.
367+
int responseCode;
369368
try {
370-
connection.connect();
371-
} catch (IOException e) {
372-
throw new BoxAPIException("Couldn't connect to the Box API due to a network error.", e);
373-
}
369+
this.writeBody(connection, listener);
374370

375-
this.logRequest(connection);
371+
// Ensure that we're connected in case writeBody() didn't write anything.
372+
try {
373+
connection.connect();
374+
} catch (IOException e) {
375+
throw new BoxAPIException("Couldn't connect to the Box API due to a network error.", e);
376+
}
376377

377-
// We need to manually handle redirects by creating a new HttpURLConnection so that connection pooling happens
378-
// correctly. There seems to be a bug in Oracle's Java implementation where automatically handled redirects will
379-
// not keep the connection alive.
380-
int responseCode;
381-
try {
382-
responseCode = connection.getResponseCode();
383-
} catch (IOException e) {
384-
throw new BoxAPIException("Couldn't connect to the Box API due to a network error.", e);
378+
this.logRequest(connection);
379+
380+
// We need to manually handle redirects by creating a new HttpURLConnection so that connection pooling
381+
// happens correctly. There seems to be a bug in Oracle's Java implementation where automatically handled
382+
// redirects will not keep the connection alive.
383+
try {
384+
responseCode = connection.getResponseCode();
385+
} catch (IOException e) {
386+
throw new BoxAPIException("Couldn't connect to the Box API due to a network error.", e);
387+
}
388+
} finally {
389+
if (this.api != null) {
390+
this.api.unlockAccessToken();
391+
}
385392
}
393+
386394
if (isResponseRedirect(responseCode)) {
387395
return this.handleRedirect(connection, listener);
388396
}

src/main/java/com/box/sdk/SharedLinkAPIConnection.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ public void refresh() {
116116
this.wrappedConnection.refresh();
117117
}
118118

119+
@Override
120+
String lockAccessToken() {
121+
return this.wrappedConnection.lockAccessToken();
122+
}
123+
124+
@Override
125+
void unlockAccessToken() {
126+
this.wrappedConnection.unlockAccessToken();
127+
}
128+
119129
/**
120130
* Gets the shared link used for accessing shared items.
121131
* @return the shared link used for accessing shared items.

0 commit comments

Comments
 (0)