Skip to content

Commit b2b8015

Browse files
adinaueradinauer
committed
Merge branch 'fix/data-collection-android-installation-id' into fix/data-collection-spring-binding
2 parents ffc725c + 2296468 commit b2b8015

5 files changed

Lines changed: 140 additions & 54 deletions

File tree

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,14 @@ private static void readDefaultOptionValues(
471471
options.addInAppInclude(packageName);
472472
}
473473
}
474+
475+
if (options.getDistinctId() == null) {
476+
try {
477+
options.setDistinctId(Installation.id(context));
478+
} catch (RuntimeException e) {
479+
options.getLogger().log(SentryLevel.ERROR, "Could not generate distinct Id.", e);
480+
}
481+
}
474482
}
475483

476484
/**

‎sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,6 @@ public static void init(
149149
"Error in the 'OptionsConfiguration.configure' callback.",
150150
t);
151151
}
152-
if (options.getDistinctId() == null
153-
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
154-
try {
155-
options.setDistinctId(Installation.id(context));
156-
} catch (RuntimeException e) {
157-
options.getLogger().log(SentryLevel.ERROR, "Could not generate distinct Id.", e);
158-
}
159-
}
160152

161153
// if SentryPerformanceProvider was disabled or removed,
162154
// we set the app start / sdk init time here instead

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt‎

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,6 @@ class AndroidOptionsInitializerTest {
111111
)
112112

113113
sentryOptions.configureOptions()
114-
if (
115-
sentryOptions.distinctId == null &&
116-
sentryOptions.dataCollectionResolver.isUserInfoWithLegacyAlways
117-
) {
118-
sentryOptions.distinctId = Installation.id(if (useRealContext) context else mockContext)
119-
}
120114
AndroidOptionsInitializer.initializeIntegrationsAndProcessors(
121115
sentryOptions,
122116
if (useRealContext) context else mockContext,
@@ -355,44 +349,6 @@ class AndroidOptionsInitializerTest {
355349
installation.deleteOnExit()
356350
}
357351

358-
@Test
359-
fun `init should not set generated distinct id when user info is disabled`() {
360-
fixture.initSut(configureOptions = { dataCollection.setUserInfo(false) })
361-
362-
assertNull(fixture.sentryOptions.distinctId)
363-
}
364-
365-
@Test
366-
fun `init should set generated distinct id when user info is enabled`() {
367-
fixture.initSut(configureOptions = { dataCollection.setUserInfo(true) })
368-
369-
assertNotNull(fixture.sentryOptions.distinctId)
370-
}
371-
372-
@Test
373-
fun `init should preserve explicit distinct id when user info is disabled`() {
374-
fixture.initSut(
375-
configureOptions = {
376-
dataCollection.setUserInfo(false)
377-
distinctId = "custom-id"
378-
}
379-
)
380-
381-
assertEquals("custom-id", fixture.sentryOptions.distinctId)
382-
}
383-
384-
@Test
385-
fun `init should set generated distinct id when explicit value is null`() {
386-
fixture.initSut(
387-
configureOptions = {
388-
dataCollection.setUserInfo(true)
389-
distinctId = null
390-
}
391-
)
392-
393-
assertNotNull(fixture.sentryOptions.distinctId)
394-
}
395-
396352
@Test
397353
fun `init should set proguard uuid id on start`() {
398354
fixture.initSut(

‎sentry/src/main/java/io/sentry/util/HttpUtils.java‎

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,75 @@ public static boolean containsSensitiveHeader(final @NotNull String header) {
208208
}
209209

210210
private static boolean isValidCookiePair(final @NotNull String cookie) {
211-
final int separator = cookie.indexOf('=');
212-
return separator >= 0 && !cookie.substring(0, separator).trim().isEmpty();
211+
final @NotNull String cookiePair = cookie.trim();
212+
final int separator = cookiePair.indexOf('=');
213+
if (separator <= 0 || !isValidCookieName(cookiePair.substring(0, separator))) {
214+
return false;
215+
}
216+
217+
final @NotNull String value = cookiePair.substring(separator + 1);
218+
int start = 0;
219+
int end = value.length();
220+
if (!value.isEmpty() && value.charAt(0) == '"') {
221+
if (value.length() < 2 || value.charAt(value.length() - 1) != '"') {
222+
return false;
223+
}
224+
start++;
225+
end--;
226+
}
227+
228+
for (int i = start; i < end; i++) {
229+
if (!isCookieOctet(value.charAt(i))) {
230+
return false;
231+
}
232+
}
233+
return true;
234+
}
235+
236+
private static boolean isValidCookieName(final @NotNull String name) {
237+
for (int i = 0; i < name.length(); i++) {
238+
if (!isCookieNameCharacter(name.charAt(i))) {
239+
return false;
240+
}
241+
}
242+
return true;
243+
}
244+
245+
private static boolean isCookieNameCharacter(final char value) {
246+
if ((value >= 'a' && value <= 'z')
247+
|| (value >= 'A' && value <= 'Z')
248+
|| (value >= '0' && value <= '9')) {
249+
return true;
250+
}
251+
252+
switch (value) {
253+
case '!':
254+
case '#':
255+
case '$':
256+
case '%':
257+
case '&':
258+
case '\'':
259+
case '*':
260+
case '+':
261+
case '-':
262+
case '.':
263+
case '^':
264+
case '_':
265+
case '`':
266+
case '|':
267+
case '~':
268+
return true;
269+
default:
270+
return false;
271+
}
272+
}
273+
274+
private static boolean isCookieOctet(final char value) {
275+
return value == 0x21
276+
|| (value >= 0x23 && value <= 0x2B)
277+
|| (value >= 0x2D && value <= 0x3A)
278+
|| (value >= 0x3C && value <= 0x5B)
279+
|| (value >= 0x5D && value <= 0x7E);
213280
}
214281

215282
public static @NotNull Map<String, String> filterHeaders(

‎sentry/src/test/java/io/sentry/util/HttpUtilsTest.kt‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,69 @@ class HttpUtilsTest {
133133
.isEqualTo("theme=dark;[Filtered];[Filtered]; empty=; sessionId=[Filtered]")
134134
}
135135

136+
@Test
137+
fun `cookie filter replaces comma-separated malformed cookies`() {
138+
assertThat(
139+
HttpUtils.filterCookies(
140+
"theme=dark, sessionId=secret",
141+
KeyValueCollectionBehavior.denyList(),
142+
emptyList(),
143+
)
144+
)
145+
.isEqualTo("[Filtered]")
146+
}
147+
148+
@Test
149+
fun `cookie filter replaces space-separated malformed cookies`() {
150+
assertThat(
151+
HttpUtils.filterCookies(
152+
"theme=dark sessionId=secret",
153+
KeyValueCollectionBehavior.denyList(),
154+
emptyList(),
155+
)
156+
)
157+
.isEqualTo("[Filtered]")
158+
}
159+
160+
@Test
161+
fun `cookie filter preserves valid names and values`() {
162+
val cookies =
163+
"plain=abc123; empty=; base64=YWJjZA==; quoted=\"dark\"; quoted-empty=\"\"; encoded=hello%2Fworld; !#\$%&'*+-.^_`|~=!#\$%&'()*+-./:<=>?@[]^_`{|}~"
164+
165+
assertThat(
166+
HttpUtils.filterCookies(
167+
cookies,
168+
KeyValueCollectionBehavior.denyList(),
169+
emptyList(),
170+
)
171+
)
172+
.isEqualTo(cookies)
173+
}
174+
175+
@Test
176+
fun `cookie filter replaces comma-separated malformed cookies in quoted values`() {
177+
assertThat(
178+
HttpUtils.filterCookies(
179+
"theme=\"dark, sessionId=secret\"",
180+
KeyValueCollectionBehavior.denyList(),
181+
emptyList(),
182+
)
183+
)
184+
.isEqualTo("[Filtered]")
185+
}
186+
187+
@Test
188+
fun `cookie filter replaces space-separated malformed cookies in quoted values`() {
189+
assertThat(
190+
HttpUtils.filterCookies(
191+
"theme=\"dark sessionId=secret\"",
192+
KeyValueCollectionBehavior.denyList(),
193+
emptyList(),
194+
)
195+
)
196+
.isEqualTo("[Filtered]")
197+
}
198+
136199
@Test
137200
fun `cookie allow list never exposes malformed pairs`() {
138201
assertThat(

0 commit comments

Comments
 (0)