Skip to content

Commit 21a214b

Browse files
adinaueradinauer
authored
Avoid NPEs from SDK in API like setTag, setData, setContext (#4245)
* Avoid NPEs from SDK in API like setTag, setData, setContext * changelog * revert unintentional change * Update CHANGELOG.md * set timeouts for okhttp in system tests * bump timeout
1 parent 5eb3279 commit 21a214b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+802
-193
lines changed

‎CHANGELOG.md‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
## Unreleased
44

5-
### Behavioural Changes
6-
7-
- Use `java.net.URI` for parsing URLs in `UrlUtils` ([#4210](https://github.com/getsentry/sentry-java/pull/4210))
8-
- This could affect grouping for issues with messages containing URLs that fall in known corner cases that were handled incorrectly previously (e.g. email in URL path)
9-
105
### Fixes
116

7+
- The SDK now handles `null` on many APIs instead of expecting a non `null` value ([#4245](https://github.com/getsentry/sentry-java/pull/4245))
8+
- Certain APIs like `setTag`, `setData`, `setExtra`, `setContext` previously caused a `NullPointerException` when invoked with either `null` key or value.
9+
- The SDK now tries to have a sane fallback when `null` is passed and no longer throws `NullPointerException`
10+
- If `null` is passed, the SDK will
11+
- do nothing if a `null` key is passed, returning `null` for non void methods
12+
- remove any previous value if the new value is set to `null`
1213
- Add support for setting in-app-includes/in-app-excludes via AndroidManifest.xml ([#4240](https://github.com/getsentry/sentry-java/pull/4240))
1314
- Modifications to OkHttp requests are now properly propagated to the affected span / breadcrumbs ([#4238](https://github.com/getsentry/sentry-java/pull/4238))
1415
- Please ensure the SentryOkHttpInterceptor is added last to your OkHttpClient, as otherwise changes to the `Request` by subsequent interceptors won't be considered
@@ -22,6 +23,11 @@
2223
- Set `sentry.capture-open-telemetry-events=true` in Springs `application.properties` to enable it
2324
- Set `sentry.captureOpenTelemetryEvents: true` in Springs `application.yml` to enable it
2425

26+
### Behavioural Changes
27+
28+
- Use `java.net.URI` for parsing URLs in `UrlUtils` ([#4210](https://github.com/getsentry/sentry-java/pull/4210))
29+
- This could affect grouping for issues with messages containing URLs that fall in known corner cases that were handled incorrectly previously (e.g. email in URL path)
30+
2531
### Internal
2632

2733
- Also use port when checking if a request is made to Sentry DSN ([#4231](https://github.com/getsentry/sentry-java/pull/4231))

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ class InternalSentrySdkTest {
319319
fun `serializeScope provides fallback app data if none is set`() {
320320
val options = SentryAndroidOptions()
321321
val scope = Scope(options)
322-
scope.setContexts("app", null)
322+
scope.setContexts("app", null as Any?)
323323

324324
val serializedScope = InternalSentrySdk.serializeScope(context, options, scope)
325325
assertTrue(((serializedScope["contexts"] as Map<*, *>)["app"] as Map<*, *>).containsKey("app_name"))

‎sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelStrongRefSpanWrapper.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,12 @@ public void setThrowable(@Nullable Throwable throwable) {
225225
}
226226

227227
@Override
228-
public void setTag(@NotNull String key, @NotNull String value) {
228+
public void setTag(@Nullable String key, @Nullable String value) {
229229
delegate.setTag(key, value);
230230
}
231231

232232
@Override
233-
public @Nullable String getTag(@NotNull String key) {
233+
public @Nullable String getTag(@Nullable String key) {
234234
return delegate.getTag(key);
235235
}
236236

@@ -240,12 +240,12 @@ public boolean isFinished() {
240240
}
241241

242242
@Override
243-
public void setData(@NotNull String key, @NotNull Object value) {
243+
public void setData(@Nullable String key, @Nullable Object value) {
244244
delegate.setData(key, value);
245245
}
246246

247247
@Override
248-
public @Nullable Object getData(@NotNull String key) {
248+
public @Nullable Object getData(@Nullable String key) {
249249
return delegate.getData(key);
250250
}
251251

@@ -281,7 +281,7 @@ public boolean isNoOp() {
281281
}
282282

283283
@Override
284-
public void setContext(@NotNull String key, @NotNull Object context) {
284+
public void setContext(@Nullable String key, @Nullable Object context) {
285285
delegate.setContext(key, context);
286286
}
287287

‎sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelTransactionSpanForwarder.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,12 @@ public void setThrowable(@Nullable Throwable throwable) {
152152
}
153153

154154
@Override
155-
public void setTag(@NotNull String key, @NotNull String value) {
155+
public void setTag(@Nullable String key, @Nullable String value) {
156156
rootSpan.setTag(key, value);
157157
}
158158

159159
@Override
160-
public @Nullable String getTag(@NotNull String key) {
160+
public @Nullable String getTag(@Nullable String key) {
161161
return rootSpan.getTag(key);
162162
}
163163

@@ -167,12 +167,12 @@ public boolean isFinished() {
167167
}
168168

169169
@Override
170-
public void setData(@NotNull String key, @NotNull Object value) {
170+
public void setData(@Nullable String key, @Nullable Object value) {
171171
rootSpan.setData(key, value);
172172
}
173173

174174
@Override
175-
public @Nullable Object getData(@NotNull String key) {
175+
public @Nullable Object getData(@Nullable String key) {
176176
return rootSpan.getData(key);
177177
}
178178

@@ -277,7 +277,7 @@ public void finish(
277277
}
278278

279279
@Override
280-
public void setContext(@NotNull String key, @NotNull Object context) {
280+
public void setContext(@Nullable String key, @Nullable Object context) {
281281
// thoughts:
282282
// - span would have to save it on global storage too since we can't add complex data to otel
283283
// span

‎sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,15 @@ public void setThrowable(@Nullable Throwable throwable) {
330330
}
331331

332332
@Override
333-
public void setTag(@NotNull String key, @NotNull String value) {
333+
public void setTag(@Nullable String key, @Nullable String value) {
334334
context.setTag(key, value);
335335
}
336336

337337
@Override
338-
public @Nullable String getTag(@NotNull String key) {
338+
public @Nullable String getTag(@Nullable String key) {
339+
if (key == null) {
340+
return null;
341+
}
339342
return context.getTags().get(key);
340343
}
341344

@@ -357,12 +360,22 @@ public boolean isFinished() {
357360
}
358361

359362
@Override
360-
public void setData(@NotNull String key, @NotNull Object value) {
361-
data.put(key, value);
363+
public void setData(@Nullable String key, @Nullable Object value) {
364+
if (key == null) {
365+
return;
366+
}
367+
if (value == null) {
368+
data.remove(key);
369+
} else {
370+
data.put(key, value);
371+
}
362372
}
363373

364374
@Override
365-
public @Nullable Object getData(@NotNull String key) {
375+
public @Nullable Object getData(@Nullable String key) {
376+
if (key == null) {
377+
return null;
378+
}
366379
return data.get(key);
367380
}
368381

@@ -422,7 +435,7 @@ public boolean isNoOp() {
422435
}
423436

424437
@Override
425-
public void setContext(@NotNull String key, @NotNull Object context) {
438+
public void setContext(@Nullable String key, @Nullable Object context) {
426439
contexts.put(key, context);
427440
}
428441

‎sentry-system-test-support/src/main/kotlin/io/sentry/systemtest/util/LoggingInsecureRestClient.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import okhttp3.RequestBody
1010
import okhttp3.RequestBody.Companion.toRequestBody
1111
import okhttp3.Response
1212
import org.slf4j.LoggerFactory
13+
import java.util.concurrent.TimeUnit
1314

1415
open class LoggingInsecureRestClient {
1516
val logger = LoggerFactory.getLogger(LoggingInsecureRestClient::class.java)
@@ -55,6 +56,10 @@ open class LoggingInsecureRestClient {
5556

5657
protected fun client(): OkHttpClient {
5758
return OkHttpClient.Builder()
59+
.callTimeout(60, TimeUnit.SECONDS)
60+
.connectTimeout(60, TimeUnit.SECONDS)
61+
.readTimeout(60, TimeUnit.SECONDS)
62+
.writeTimeout(60, TimeUnit.SECONDS)
5863
.build()
5964
}
6065

‎sentry/api/sentry.api‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,11 @@ public final class io/sentry/CombinedContextsView : io/sentry/protocol/Contexts
238238
public fun isEmpty ()Z
239239
public fun keys ()Ljava/util/Enumeration;
240240
public fun put (Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;
241+
public fun putAll (Lio/sentry/protocol/Contexts;)V
242+
public fun putAll (Ljava/util/Map;)V
241243
public fun remove (Ljava/lang/Object;)Ljava/lang/Object;
242244
public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V
245+
public fun set (Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;
243246
public fun setApp (Lio/sentry/protocol/App;)V
244247
public fun setBrowser (Lio/sentry/protocol/Browser;)V
245248
public fun setDevice (Lio/sentry/protocol/Device;)V

‎sentry/src/main/java/io/sentry/Breadcrumb.java‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,10 @@ public Map<String, Object> getData() {
616616
* @return the value or null
617617
*/
618618
@Nullable
619-
public Object getData(final @NotNull String key) {
619+
public Object getData(final @Nullable String key) {
620+
if (key == null) {
621+
return null;
622+
}
620623
return data.get(key);
621624
}
622625

@@ -626,16 +629,26 @@ public Object getData(final @NotNull String key) {
626629
* @param key the key
627630
* @param value the value
628631
*/
629-
public void setData(@NotNull String key, @NotNull Object value) {
630-
data.put(key, value);
632+
public void setData(@Nullable String key, @Nullable Object value) {
633+
if (key == null) {
634+
return;
635+
}
636+
if (value == null) {
637+
removeData(key);
638+
} else {
639+
data.put(key, value);
640+
}
631641
}
632642

633643
/**
634644
* Removes an entry from the data's map
635645
*
636646
* @param key the key
637647
*/
638-
public void removeData(@NotNull String key) {
648+
public void removeData(@Nullable String key) {
649+
if (key == null) {
650+
return;
651+
}
639652
data.remove(key);
640653
}
641654

‎sentry/src/main/java/io/sentry/CombinedContextsView.java‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,14 +241,14 @@ public boolean isEmpty() {
241241
}
242242

243243
@Override
244-
public boolean containsKey(final @NotNull Object key) {
244+
public boolean containsKey(final @Nullable Object key) {
245245
return globalContexts.containsKey(key)
246246
|| isolationContexts.containsKey(key)
247247
|| currentContexts.containsKey(key);
248248
}
249249

250250
@Override
251-
public @Nullable Object get(final @NotNull Object key) {
251+
public @Nullable Object get(final @Nullable Object key) {
252252
final @Nullable Object current = currentContexts.get(key);
253253
if (current != null) {
254254
return current;
@@ -261,12 +261,12 @@ public boolean containsKey(final @NotNull Object key) {
261261
}
262262

263263
@Override
264-
public @Nullable Object put(final @NotNull String key, final @Nullable Object value) {
264+
public @Nullable Object put(final @Nullable String key, final @Nullable Object value) {
265265
return getDefaultContexts().put(key, value);
266266
}
267267

268268
@Override
269-
public @Nullable Object remove(final @NotNull Object key) {
269+
public @Nullable Object remove(final @Nullable Object key) {
270270
return getDefaultContexts().remove(key);
271271
}
272272

@@ -281,10 +281,26 @@ public boolean containsKey(final @NotNull Object key) {
281281
}
282282

283283
@Override
284-
public void serialize(@NotNull ObjectWriter writer, @NotNull ILogger logger) throws IOException {
284+
public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger logger)
285+
throws IOException {
285286
mergeContexts().serialize(writer, logger);
286287
}
287288

289+
@Override
290+
public @Nullable Object set(@Nullable String key, @Nullable Object value) {
291+
return put(key, value);
292+
}
293+
294+
@Override
295+
public void putAll(@Nullable Map<? extends String, ?> m) {
296+
getDefaultContexts().putAll(m);
297+
}
298+
299+
@Override
300+
public void putAll(@Nullable Contexts contexts) {
301+
getDefaultContexts().putAll(contexts);
302+
}
303+
288304
private @NotNull Contexts mergeContexts() {
289305
final @NotNull Contexts allContexts = new Contexts();
290306
allContexts.putAll(globalContexts);

‎sentry/src/main/java/io/sentry/CombinedScopeView.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,12 @@ public void clear() {
229229
}
230230

231231
@Override
232-
public void setTag(@NotNull String key, @NotNull String value) {
232+
public void setTag(@Nullable String key, @Nullable String value) {
233233
getDefaultWriteScope().setTag(key, value);
234234
}
235235

236236
@Override
237-
public void removeTag(@NotNull String key) {
237+
public void removeTag(@Nullable String key) {
238238
getDefaultWriteScope().removeTag(key);
239239
}
240240

@@ -248,12 +248,12 @@ public void removeTag(@NotNull String key) {
248248
}
249249

250250
@Override
251-
public void setExtra(@NotNull String key, @NotNull String value) {
251+
public void setExtra(@Nullable String key, @Nullable String value) {
252252
getDefaultWriteScope().setExtra(key, value);
253253
}
254254

255255
@Override
256-
public void removeExtra(@NotNull String key) {
256+
public void removeExtra(@Nullable String key) {
257257
getDefaultWriteScope().removeExtra(key);
258258
}
259259

@@ -267,42 +267,42 @@ public void removeExtra(@NotNull String key) {
267267
}
268268

269269
@Override
270-
public void setContexts(@NotNull String key, @NotNull Object value) {
270+
public void setContexts(@Nullable String key, @Nullable Object value) {
271271
getDefaultWriteScope().setContexts(key, value);
272272
}
273273

274274
@Override
275-
public void setContexts(@NotNull String key, @NotNull Boolean value) {
275+
public void setContexts(@Nullable String key, @Nullable Boolean value) {
276276
getDefaultWriteScope().setContexts(key, value);
277277
}
278278

279279
@Override
280-
public void setContexts(@NotNull String key, @NotNull String value) {
280+
public void setContexts(@Nullable String key, @Nullable String value) {
281281
getDefaultWriteScope().setContexts(key, value);
282282
}
283283

284284
@Override
285-
public void setContexts(@NotNull String key, @NotNull Number value) {
285+
public void setContexts(@Nullable String key, @Nullable Number value) {
286286
getDefaultWriteScope().setContexts(key, value);
287287
}
288288

289289
@Override
290-
public void setContexts(@NotNull String key, @NotNull Collection<?> value) {
290+
public void setContexts(@Nullable String key, @Nullable Collection<?> value) {
291291
getDefaultWriteScope().setContexts(key, value);
292292
}
293293

294294
@Override
295-
public void setContexts(@NotNull String key, @NotNull Object[] value) {
295+
public void setContexts(@Nullable String key, @Nullable Object[] value) {
296296
getDefaultWriteScope().setContexts(key, value);
297297
}
298298

299299
@Override
300-
public void setContexts(@NotNull String key, @NotNull Character value) {
300+
public void setContexts(@Nullable String key, @Nullable Character value) {
301301
getDefaultWriteScope().setContexts(key, value);
302302
}
303303

304304
@Override
305-
public void removeContexts(@NotNull String key) {
305+
public void removeContexts(@Nullable String key) {
306306
getDefaultWriteScope().removeContexts(key);
307307
}
308308

0 commit comments

Comments
 (0)