Skip to content

Commit

Permalink
bug(config): move apikey validation check after bugsnag.start
Browse files Browse the repository at this point in the history
  • Loading branch information
YYChen01988 committed Jul 17, 2023
1 parent fd2b155 commit d3ea230
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import androidx.annotation.IntRange;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;

import java.io.File;
import java.util.Map;
Expand All @@ -20,7 +19,6 @@ public class Configuration implements CallbackAware, MetadataAware, UserAware, F

private static final int MIN_BREADCRUMBS = 0;
private static final int MAX_BREADCRUMBS = 500;
private static final int VALID_API_KEY_LEN = 32;
private static final long MIN_LAUNCH_CRASH_THRESHOLD_MS = 0;

final ConfigInternal impl;
Expand All @@ -29,7 +27,6 @@ public class Configuration implements CallbackAware, MetadataAware, UserAware, F
* Constructs a new Configuration object with default values.
*/
public Configuration(@NonNull String apiKey) {
validateApiKey(apiKey);
impl = new ConfigInternal(apiKey);
}

Expand All @@ -47,32 +44,6 @@ static Configuration load(@NonNull Context context, @NonNull String apiKey) {
return ConfigInternal.load(context, apiKey);
}

private void validateApiKey(String value) {
if (isInvalidApiKey(value)) {
DebugLogger.INSTANCE.w("Invalid configuration. "
+ "apiKey should be a 32-character hexademical string, got " + value);
}
}

@VisibleForTesting
static boolean isInvalidApiKey(String apiKey) {
if (Intrinsics.isEmpty(apiKey)) {
throw new IllegalArgumentException("No Bugsnag API Key set");
}
if (apiKey.length() != VALID_API_KEY_LEN) {
return true;
}
// check whether each character is hexadecimal (either a digit or a-f).
// this avoids using a regex to improve startup performance.
for (int k = 0; k < VALID_API_KEY_LEN; k++) {
char chr = apiKey.charAt(k);
if (!Character.isDigit(chr) && (chr < 'a' || chr > 'f')) {
return true;
}
}
return false;
}

private void logNull(String property) {
getLogger().e("Invalid null value supplied to config." + property + ", ignoring");
}
Expand All @@ -89,7 +60,6 @@ public String getApiKey() {
* Changes the API key used for events sent to Bugsnag.
*/
public void setApiKey(@NonNull String apiKey) {
validateApiKey(apiKey);
impl.setApiKey(apiKey);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.bugsnag.android.BreadcrumbType
import com.bugsnag.android.Configuration
import com.bugsnag.android.Connectivity
import com.bugsnag.android.DebugLogger
import com.bugsnag.android.DebugLogger.w
import com.bugsnag.android.DefaultDelivery
import com.bugsnag.android.Delivery
import com.bugsnag.android.DeliveryParams
Expand Down Expand Up @@ -176,6 +177,34 @@ internal fun convertToImmutableConfig(
)
}

private fun validateApiKey(value: String) {
if (isInvalidApiKey(value)) {
w(
"Invalid configuration. "
+ "apiKey should be a 32-character hexademical string, got " + value
)
}
}

@VisibleForTesting
fun isInvalidApiKey(apiKey: String): Boolean {
if (apiKey.isNullOrEmpty()) {
throw IllegalArgumentException("No Bugsnag API Key set")
}
if (apiKey.length != VALID_API_KEY_LEN) {
return true
}
// check whether each character is hexadecimal (either a digit or a-f).
// this avoids using a regex to improve startup performance.
for (k in 0 until VALID_API_KEY_LEN) {
val chr = apiKey[k]
if (!Character.isDigit(chr) && (chr < 'a' || chr > 'f')) {
return true
}
}
return false
}

internal fun sanitiseConfiguration(
appContext: Context,
configuration: Configuration,
Expand All @@ -188,6 +217,8 @@ internal fun sanitiseConfiguration(
packageManager.getApplicationInfo(packageName, PackageManager.GET_META_DATA)
}.getOrNull()

validateApiKey(configuration.apiKey)

// populate releaseStage
if (configuration.releaseStage == null) {
configuration.releaseStage = when {
Expand Down Expand Up @@ -251,3 +282,4 @@ private fun populateBuildUuid(appInfo: ApplicationInfo?): String? {

internal const val RELEASE_STAGE_DEVELOPMENT = "development"
internal const val RELEASE_STAGE_PRODUCTION = "production"
internal const val VALID_API_KEY_LEN = 32
Original file line number Diff line number Diff line change
@@ -1,37 +1,33 @@
package com.bugsnag.android

import com.bugsnag.android.internal.isInvalidApiKey
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test

class ApiKeyValidationTest {

@Test(expected = IllegalArgumentException::class)
fun testNullApiKey() {
Configuration.isInvalidApiKey(null)
}
internal class ApiKeyValidationTest {

@Test(expected = IllegalArgumentException::class)
fun testEmptyApiKey() {
Configuration.isInvalidApiKey("")
isInvalidApiKey("")
}

@Test
fun testWrongSizeApiKey() {
assertTrue(Configuration.isInvalidApiKey("abfe05f"))
assertTrue(Configuration.isInvalidApiKey("5d1ec5bd39a74caa1267142706a7fb212"))
assertTrue(isInvalidApiKey("abfe05f"))
assertTrue(isInvalidApiKey("5d1ec5bd39a74caa1267142706a7fb212"))
}

@Test
fun testSettingNonHexApiKey() {
assertTrue(Configuration.isInvalidApiKey("5d1ec5bd3Ga74caa1267142706a7fb21"))
assertTrue(Configuration.isInvalidApiKey("5d1ec5bd39a7%caa1267_42706P7fb212"))
assertFalse(Configuration.isInvalidApiKey("5d1ec5bd39a74caa1267142706a7fb21"))
assertTrue(isInvalidApiKey("5d1ec5bd3Ga74caa1267142706a7fb21"))
assertTrue(isInvalidApiKey("5d1ec5bd39a7%caa1267_42706P7fb212"))
assertFalse(isInvalidApiKey("5d1ec5bd39a74caa1267142706a7fb21"))
}

@Test
fun setApiKey() {
assertFalse(Configuration.isInvalidApiKey("5d1ec5bd39a74caa1267142706a7fb21"))
assertFalse(Configuration.isInvalidApiKey("000005bd39a74caa1267142706a7fb21"))
assertFalse(isInvalidApiKey("5d1ec5bd39a74caa1267142706a7fb21"))
assertFalse(isInvalidApiKey("000005bd39a74caa1267142706a7fb21"))
}
}

0 comments on commit d3ea230

Please sign in to comment.