Skip to content

Commit b1ed1f3

Browse files
Copilotfadidurah
Copilot
and
fadidurah
authored
Replace fromJson with fromPipeDelimited in token handler; remove fromJson and related tests
Agent-Logs-Url: https://github.com/AzureAD/microsoft-authentication-library-common-for-android/sessions/b86d2241-7b2c-4028-8a88-419ebf6b00ae Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
1 parent 3eaa693 commit b1ed1f3

4 files changed

Lines changed: 14 additions & 135 deletions

File tree

‎common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/AbstractMicrosoftStsTokenResponseHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public TokenResult handleTokenResponse(@NonNull final HttpResponse response) thr
109109
}
110110

111111
final String clientDataHeader = response.getHeaderValue(X_MS_CLIENTDATA, 0);
112-
final ClientDataInfo clientDataInfo = ClientDataInfo.fromJson(clientDataHeader);
112+
final ClientDataInfo clientDataInfo = ClientDataInfo.fromPipeDelimited(clientDataHeader);
113113
if (null != clientDataInfo) {
114114
clientDataInfo.emitToSpan();
115115
}

‎common4j/src/main/com/microsoft/identity/common/java/telemetry/ClientDataInfo.java‎

Lines changed: 10 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,16 @@
2727
import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
2828
import com.microsoft.identity.common.java.util.StringUtil;
2929

30-
import org.json.JSONObject;
31-
3230
import edu.umd.cs.findbugs.annotations.Nullable;
31+
import io.opentelemetry.api.trace.Span;
3332
import lombok.Getter;
3433
import lombok.Setter;
3534
import lombok.experimental.Accessors;
3635

3736
/**
38-
* Model representing the x-ms-clientdata server telemetry header (from /token responses)
39-
* and the clientdata query parameter (from /authorize redirect URLs).
37+
* Model representing server telemetry data from the x-ms-clientdata response header
38+
* (/token responses) and the clientdata query parameter (/authorize redirect URLs).
39+
* Both use a pipe-delimited format: account_type|error|sub_error|caller_data_boundary|cloud_instance.
4040
* Contains server-side error codes, account type, cloud instance, and data boundary info.
4141
*/
4242
@Getter
@@ -49,21 +49,6 @@ public class ClientDataInfo {
4949
/** Maximum length for any individual field when emitting to a span. */
5050
private static final int MAX_FIELD_LENGTH = 256;
5151

52-
/** JSON key for server error code. */
53-
private static final String JSON_KEY_ERROR = "Error";
54-
55-
/** JSON key for server sub-error code. */
56-
private static final String JSON_KEY_SUB_ERROR = "SubError";
57-
58-
/** JSON key for account type. */
59-
private static final String JSON_KEY_ACCOUNT_TYPE = "AccountType";
60-
61-
/** JSON key for cloud instance. */
62-
private static final String JSON_KEY_CLOUD_INSTANCE = "cloud_instance";
63-
64-
/** JSON key for caller data boundary. */
65-
private static final String JSON_KEY_CALLER_DATA_BOUNDARY = "caller_data_boundary";
66-
6752
/** Account type value for MSA accounts. */
6853
private static final String ACCOUNT_TYPE_MSA_RAW = "m";
6954

@@ -93,35 +78,6 @@ public class ClientDataInfo {
9378
private String mCloudInstance;
9479
private String mCallerDataBoundary;
9580

96-
/**
97-
* Parses a URL-encoded JSON x-ms-clientdata value.
98-
* Keys: Error, SubError, AccountType, cloud_instance, caller_data_boundary.
99-
*
100-
* @param urlEncodedJson URL-encoded JSON string, may be null.
101-
* @return parsed {@link ClientDataInfo}, or null on failure/empty input.
102-
*/
103-
@Nullable
104-
public static ClientDataInfo fromJson(@Nullable final String urlEncodedJson) {
105-
if (StringUtil.isNullOrEmpty(urlEncodedJson)) {
106-
return null;
107-
}
108-
try {
109-
final String decoded = StringUtil.urlFormDecode(urlEncodedJson);
110-
final JSONObject json = new JSONObject(decoded);
111-
112-
final ClientDataInfo info = new ClientDataInfo();
113-
info.mError = optString(json, JSON_KEY_ERROR);
114-
info.mSubError = optString(json, JSON_KEY_SUB_ERROR);
115-
info.mAccountType = optString(json, JSON_KEY_ACCOUNT_TYPE);
116-
info.mCloudInstance = optString(json, JSON_KEY_CLOUD_INSTANCE);
117-
info.mCallerDataBoundary = optString(json, JSON_KEY_CALLER_DATA_BOUNDARY);
118-
return info;
119-
} catch (final Exception e) {
120-
Logger.warn(TAG, "Failed to parse x-ms-clientdata JSON: " + e.getMessage());
121-
return null;
122-
}
123-
}
124-
12581
/**
12682
* Parses an already-decoded pipe-delimited clientdata query parameter value.
12783
* The caller is responsible for URL-decoding before passing (e.g. values from
@@ -167,27 +123,23 @@ public static ClientDataInfo fromPipeDelimited(@Nullable final String decodedVal
167123
* Each field is truncated to {@value #MAX_FIELD_LENGTH} characters.
168124
*/
169125
public void emitToSpan() {
126+
final Span span = SpanExtension.current();
170127
if (mError != null) {
171-
SpanExtension.current().setAttribute(
172-
AttributeName.server_error.name(), truncate(mError));
128+
span.setAttribute(AttributeName.server_error.name(), truncate(mError));
173129
}
174130
if (mSubError != null) {
175-
SpanExtension.current().setAttribute(
176-
AttributeName.server_sub_error.name(), truncate(mSubError));
131+
span.setAttribute(AttributeName.server_sub_error.name(), truncate(mSubError));
177132
}
178133
if (mAccountType != null) {
179134
// account_type is an existing AttributeName; reuse it (m -> MSA, e -> AAD).
180135
final String mappedAccountType = mapAccountType(mAccountType);
181-
SpanExtension.current().setAttribute(
182-
AttributeName.account_type.name(), truncate(mappedAccountType));
136+
span.setAttribute(AttributeName.account_type.name(), truncate(mappedAccountType));
183137
}
184138
if (mCloudInstance != null) {
185-
SpanExtension.current().setAttribute(
186-
AttributeName.server_cloud_instance.name(), truncate(mCloudInstance));
139+
span.setAttribute(AttributeName.server_cloud_instance.name(), truncate(mCloudInstance));
187140
}
188141
if (mCallerDataBoundary != null) {
189-
SpanExtension.current().setAttribute(
190-
AttributeName.server_caller_data_boundary.name(), truncate(mCallerDataBoundary));
142+
span.setAttribute(AttributeName.server_caller_data_boundary.name(), truncate(mCallerDataBoundary));
191143
}
192144
}
193145

@@ -208,15 +160,6 @@ private static String truncate(final String value) {
208160
return value.substring(0, MAX_FIELD_LENGTH);
209161
}
210162

211-
@Nullable
212-
private static String optString(final JSONObject json, final String key) {
213-
if (json.isNull(key)) {
214-
return null;
215-
}
216-
final String value = json.optString(key, null);
217-
return StringUtil.isNullOrEmpty(value) ? null : value;
218-
}
219-
220163
@Nullable
221164
private static String emptyToNull(final String value) {
222165
return StringUtil.isNullOrEmpty(value) ? null : value;

‎common4j/src/test/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsTokenResponseHandlerTest.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,13 @@
2929
import com.microsoft.identity.common.java.providers.microsoft.MicrosoftTokenErrorResponse;
3030
import com.microsoft.identity.common.java.providers.oauth2.TokenResult;
3131

32-
import org.junit.After;
3332
import org.junit.Assert;
3433
import org.junit.Test;
3534
import org.junit.runner.RunWith;
3635
import org.junit.runners.JUnit4;
3736
import org.mockito.MockedStatic;
3837
import org.mockito.Mockito;
3938

40-
import java.net.URLEncoder;
41-
import java.nio.charset.StandardCharsets;
4239
import java.util.Collections;
4340
import java.util.HashMap;
4441
import java.util.List;
@@ -106,17 +103,13 @@ public void testHandleTokenResponse_Error() {
106103
@SneakyThrows
107104
@Test
108105
public void testHandleTokenResponse_withClientDataHeader_attributesEmitted() {
109-
final String rawJson = "{\"Error\":\"AADSTS50058\","
110-
+ "\"SubError\":\"login_required\","
111-
+ "\"AccountType\":\"e\","
112-
+ "\"cloud_instance\":\"public\","
113-
+ "\"caller_data_boundary\":\"us\"}";
114-
final String encodedHeader = URLEncoder.encode(rawJson, StandardCharsets.UTF_8.name());
106+
// Header value is a pipe-delimited string: account_type|error|sub_error|caller_data_boundary|cloud_instance
107+
final String clientDataHeader = "e|AADSTS50058|login_required|us|public";
115108

116109
final HashMap<String, List<String>> headers = new HashMap<>();
117110
headers.put("Content-Type", Collections.singletonList("application/json; charset=utf-8"));
118111
headers.put(HttpConstants.HeaderField.X_MS_CLIENTDATA,
119-
Collections.singletonList(encodedHeader));
112+
Collections.singletonList(clientDataHeader));
120113

121114
final HttpResponse response = new HttpResponse(200, MOCK_TOKEN_SUCCESS_RESPONSE, headers);
122115
final MicrosoftStsTokenResponseHandler handler = new MicrosoftStsTokenResponseHandler();

‎common4j/src/test/com/microsoft/identity/common/java/telemetry/ClientDataInfoTest.java‎

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@
3131
import org.mockito.MockedStatic;
3232
import org.mockito.Mockito;
3333

34-
import java.net.URLEncoder;
35-
import java.nio.charset.StandardCharsets;
36-
3734
import io.opentelemetry.api.trace.Span;
3835

3936
import static org.junit.Assert.assertEquals;
@@ -50,60 +47,6 @@
5047
@RunWith(JUnit4.class)
5148
public class ClientDataInfoTest {
5249

53-
// -------------------------------------------------------------------------
54-
// fromJson() tests
55-
// -------------------------------------------------------------------------
56-
57-
@Test
58-
public void fromJson_validInput_allFieldsParsed() throws Exception {
59-
final String raw = "{\"Error\":\"AADSTS50058\","
60-
+ "\"SubError\":\"login_required\","
61-
+ "\"AccountType\":\"m\","
62-
+ "\"cloud_instance\":\"public\","
63-
+ "\"caller_data_boundary\":\"us\"}";
64-
final String encoded = URLEncoder.encode(raw, StandardCharsets.UTF_8.name());
65-
66-
final ClientDataInfo info = ClientDataInfo.fromJson(encoded);
67-
68-
assertNotNull(info);
69-
assertEquals("AADSTS50058", info.getError());
70-
assertEquals("login_required", info.getSubError());
71-
assertEquals("m", info.getAccountType());
72-
assertEquals("public", info.getCloudInstance());
73-
assertEquals("us", info.getCallerDataBoundary());
74-
}
75-
76-
@Test
77-
public void fromJson_partialInput_onlyPresentFieldsSet() throws Exception {
78-
final String raw = "{\"Error\":\"AADSTS65001\",\"AccountType\":\"e\"}";
79-
final String encoded = URLEncoder.encode(raw, StandardCharsets.UTF_8.name());
80-
81-
final ClientDataInfo info = ClientDataInfo.fromJson(encoded);
82-
83-
assertNotNull(info);
84-
assertEquals("AADSTS65001", info.getError());
85-
assertEquals("e", info.getAccountType());
86-
assertNull(info.getSubError());
87-
assertNull(info.getCloudInstance());
88-
assertNull(info.getCallerDataBoundary());
89-
}
90-
91-
@Test
92-
public void fromJson_malformedJson_returnsNull() throws Exception {
93-
final String malformed = URLEncoder.encode("{not valid json}", StandardCharsets.UTF_8.name());
94-
assertNull(ClientDataInfo.fromJson(malformed));
95-
}
96-
97-
@Test
98-
public void fromJson_nullInput_returnsNull() {
99-
assertNull(ClientDataInfo.fromJson(null));
100-
}
101-
102-
@Test
103-
public void fromJson_emptyString_returnsNull() {
104-
assertNull(ClientDataInfo.fromJson(""));
105-
}
106-
10750
// -------------------------------------------------------------------------
10851
// fromPipeDelimited() tests
10952
// -------------------------------------------------------------------------

0 commit comments

Comments
 (0)