From 7b99194f9309f4dc47f11caa8ae2ed03d2e145f3 Mon Sep 17 00:00:00 2001 From: Abhishek Bagde Date: Sat, 25 Apr 2026 13:36:41 +0000 Subject: [PATCH] fix: ResponseHandler.getResponse() returns OK after 401 (issue #107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When ErrorResponseHandler built the UnAuthorizedException message it called context.request().getRequestUri() on the RequestInformation object. That method calls LocationService.getInstance().getLocation(), which creates a *fresh* LocationService (empty cache) and makes an OPTIONS request to discover the resource area. That OPTIONS call goes through the full response pipeline, causing ApiResponseHandler to overwrite ResponseHandler.apiResponse with the 200 OK OPTIONS response — hiding the real 401 from the caller. Fix: replace context.request().getRequestUri() with context.response().request().uri(), which reads the URI directly from the already-sent HttpResponse with no side effects. Regression test added in WorkItemTrackingRequestBuilderTest: shouldRetainUnauthorizedStatusInResponseHandlerWhenProjectIsAbsent --- CHANGELOG.md | 6 +++ .../handlers/ErrorResponseHandler.java | 6 ++- .../WorkItemTrackingRequestBuilderTest.java | 42 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c539dbf..04c9404a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +# 7.0.1 + +- Fixed issues: + - Issue: [attempting to get WorkItemItemTypes without a project causes an UnAuthorizedException but the http code is OK #107](https://github.com/hkarthik7/azure-devops-java-sdk/issues/107) + - `ErrorResponseHandler` was calling `context.request().getRequestUri()` to build the 401 error message. This triggered a fresh `LocationService` OPTIONS lookup that overwrote `ResponseHandler.apiResponse` with 200 OK, hiding the real 401 from callers of `ResponseHandler.getResponse()`. Fixed by using `context.response().request().uri()` (the URI from the already-sent `HttpResponse`) instead. + # 7.0.0 **Major release** diff --git a/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java b/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java index 93955381..902d86d9 100644 --- a/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java +++ b/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java @@ -36,11 +36,15 @@ public CompletableFuture handleAsync(ResponseContext context) { } if (status == 401) { + // Use the URI from the already-sent HttpResponse rather than calling + // context.request().getRequestUri(), which triggers a fresh LocationService + // lookup (OPTIONS request) that overwrites ResponseHandler.apiResponse with + // 200 OK — hiding the real 401 from callers of ResponseHandler.getResponse(). return CompletableFuture.failedFuture( new AzDException( ApiExceptionTypes.UnAuthorizedException.toString(), "Given token doesn't have access to resource '" - + context.request().getRequestUri() + "'." + + context.response().request().uri() + "'." ) ); } diff --git a/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java b/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java index bbd96308..362cfe01 100644 --- a/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java +++ b/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java @@ -7,6 +7,8 @@ import org.azd.authentication.PersonalAccessTokenCredential; import org.azd.common.types.JsonPatchDocument; import org.azd.enums.*; +import org.azd.abstractions.ResponseHandler; +import org.azd.enums.HttpStatusCode; import org.azd.exceptions.AzDException; import org.azd.helpers.StreamHelper; import org.azd.http.ClientRequest; @@ -24,6 +26,8 @@ import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; public class WorkItemTrackingRequestBuilderTest { private static final SerializerContext serializer = InstanceFactory.createSerializerContext(); @@ -456,4 +460,42 @@ public void shouldUpdateAFormattedCommentForGivenWorkItem() throws AzDException w.comments().update("# This is an updated comment.", comment.getId(), 2177, CommentFormat.markdown); } + + /** + * Regression test for issue #107. + * + * When a 401 is returned (e.g. calling workItemTypes().list() without a project), + * ResponseHandler.getResponse() must still reflect the 401 status after the exception + * is thrown. Previously, ErrorResponseHandler called context.request().getRequestUri() + * to build the error message, which triggered a fresh LocationService OPTIONS call that + * overwrote ResponseHandler.apiResponse with 200 OK before the exception reached the + * caller. + */ + @Test + public void shouldRetainUnauthorizedStatusInResponseHandlerWhenProjectIsAbsent() { + String dir = System.getProperty("user.dir"); + var file = new File(dir + "/src/test/java/org/azd/_unitTest.json"); + MockParameters m; + try { + m = InstanceFactory.createSerializerContext().deserialize(file, MockParameters.class); + } catch (AzDException e) { + return; // skip if config not available + } + // Build a client WITHOUT project — mirrors the reporter's code in issue #107. + var pat = new PersonalAccessTokenCredential( + Instance.BASE_INSTANCE.append(m.getO()), m.getT()); + var noProjectClient = AzDService.builder().authentication(pat).buildClient(); + + try { + noProjectClient.workItemTracking().workItemTypes().list(); + } catch (AzDException e) { + // Exception is expected. The key assertion: ResponseHandler must NOT have + // been overwritten with 200 OK by the internal location-discovery request. + var response = ResponseHandler.getResponse(); + assertNotNull("ResponseHandler.getResponse() must not be null after a failed call", response); + assertNotEquals( + "ResponseHandler.getResponse() must not be OK after a 401 — issue #107", + HttpStatusCode.OK, response.getStatusCode()); + } + } }