Skip to content

Commit e055dfd

Browse files
committed
feedback
1 parent 79ba691 commit e055dfd

File tree

4 files changed

+67
-74
lines changed

4 files changed

+67
-74
lines changed

plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/credentials/sso/DiskCache.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ import software.aws.toolkits.core.utils.touch
3232
import software.aws.toolkits.core.utils.tryDirOp
3333
import software.aws.toolkits.core.utils.tryFileOp
3434
import software.aws.toolkits.core.utils.tryOrNull
35-
import software.aws.toolkits.jetbrains.services.telemetry.scrubNames
3635
import software.aws.toolkits.core.utils.warn
36+
import software.aws.toolkits.jetbrains.services.telemetry.scrubNames
3737
import software.aws.toolkits.telemetry.AuthTelemetry
3838
import software.aws.toolkits.telemetry.Result
3939
import java.io.InputStream

plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/credentials/sso/SsoAccessTokenProvider.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
package software.aws.toolkits.jetbrains.core.credentials.sso
55

6-
import com.intellij.openapi.application.ApplicationInfo
76
import com.intellij.openapi.components.service
87
import com.intellij.openapi.progress.EmptyProgressIndicator
98
import com.intellij.openapi.progress.ProcessCanceledException
@@ -347,7 +346,6 @@ class SsoAccessTokenProvider(
347346

348347
onPendingToken.tokenRetrieved()
349348
_authorization.set(null)
350-
ApplicationInfo.getInstance().build.baselineVersion
351349

352350
return tokenResponse.toDAGAccessToken(authorization.createdAt)
353351
} catch (e: SlowDownException) {

plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/services/telemetry/TelemetryUtils.kt

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -201,17 +201,8 @@ val ALLOWED_CODE_EXTENSIONS = setOf(
201201
"zig"
202202
)
203203

204-
fun scrubNames(messageToBeScrubbed: String, username: String? = null): String {
204+
fun scrubNames(messageToBeScrubbed: String, username: String? = getSystemUserName()): String {
205205
var scrubbedMessage = ""
206-
val fileExtRegex = Regex("""\.[^./]+$""")
207-
val slashdot = Regex("""^[~.]*[/\\]*""")
208-
209-
/** Allowlisted filepath segments. */
210-
val keep = setOf(
211-
"~", ".", "..", ".aws", "aws", "sso", "cache", "credentials", "config",
212-
"Users", "users", "home", "tmp", "aws-toolkit-jetbrains"
213-
)
214-
215206
var processedMessage = messageToBeScrubbed
216207
if (!username.isNullOrEmpty() && username.length > 2) {
217208
processedMessage = processedMessage.replace(username, "x")
@@ -240,11 +231,11 @@ fun scrubNames(messageToBeScrubbed: String, username: String? = null): String {
240231

241232
val ps = pathSegments.filterIndexed { i, _ -> i != 0 }.toMutableList()
242233
ps.add(0, firstVal)
243-
val driveLetterRegex = Regex("""^[a-zA-Z]:""")
234+
244235
for (seg in ps) {
245236
when {
246237
driveLetterRegex.matches(seg) -> scrubbed += seg
247-
keep.contains(seg) -> scrubbed += "/$seg"
238+
commonFilePathPatterns.contains(seg) -> scrubbed += "/$seg"
248239
else -> {
249240
// Save the first non-ASCII (unicode) char, if any.
250241
val nonAscii = Regex("""[^\p{ASCII}]""").find(seg)?.value ?: ""
@@ -263,3 +254,15 @@ fun scrubNames(messageToBeScrubbed: String, username: String? = null): String {
263254

264255
return scrubbedMessage.trim()
265256
}
257+
258+
val fileExtRegex = Regex("""\.[^./]+$""")
259+
val slashdot = Regex("""^[~.]*[/\\]*""")
260+
261+
/** Allowlisted filepath segments. */
262+
val commonFilePathPatterns = setOf(
263+
"~", ".", "..", ".aws", "aws", "sso", "cache", "credentials", "config",
264+
"Users", "users", "home", "tmp", "aws-toolkit-jetbrains"
265+
)
266+
val driveLetterRegex = Regex("""^[a-zA-Z]:""")
267+
268+
fun getSystemUserName(): String? = System.getProperty("user.name") ?: null

plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/telemetry/TelemetryUtilsTest.kt

Lines changed: 51 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -3,72 +3,64 @@
33

44
package software.aws.toolkits.jetbrains.services.telemetry
55

6-
import org.junit.jupiter.api.Assertions.assertEquals
7-
import org.junit.jupiter.api.Test
6+
import org.assertj.core.api.Assertions.assertThat
7+
import org.junit.jupiter.params.ParameterizedTest
8+
import org.junit.jupiter.params.provider.Arguments
9+
import org.junit.jupiter.params.provider.MethodSource
10+
import java.util.stream.Stream
811

912
class TelemetryUtilsTest {
1013

11-
@Test
12-
fun testScrubNames() {
14+
@ParameterizedTest
15+
@MethodSource("scrubNamesTestCases")
16+
fun testScrubNames(input: String, expected: String) {
1317
val fakeUser = "jdoe123"
18+
assertThat(expected).isEqualTo(scrubNames(input, fakeUser))
19+
}
1420

15-
assertEquals("", scrubNames("", fakeUser))
16-
assertEquals("a ./ b", scrubNames("a ./ b", fakeUser))
17-
assertEquals("a ../ b", scrubNames("a ../ b", fakeUser))
18-
assertEquals("a /.. b", scrubNames("a /.. b", fakeUser))
19-
assertEquals("a //..// b", scrubNames("a //..// b", fakeUser))
20-
assertEquals("a / b", scrubNames("a / b", fakeUser))
21-
assertEquals("a ~/ b", scrubNames("a ~/ b", fakeUser))
22-
assertEquals("a //// b", scrubNames("a //// b", fakeUser))
23-
assertEquals("a .. b", scrubNames("a .. b", fakeUser))
24-
assertEquals("a . b", scrubNames("a . b", fakeUser))
25-
assertEquals("lots of x", scrubNames(" lots of space ", "space"))
26-
assertEquals(
27-
"Failed to save c:/xß/xï/xó/x∑/xπ/xö/x/xö/x.txt no permissions (error!)",
28-
scrubNames(
21+
companion object {
22+
@JvmStatic
23+
fun scrubNamesTestCases(): Stream<Arguments> = Stream.of(
24+
Arguments.of("", ""),
25+
Arguments.of("a ./ b", "a ./ b"),
26+
Arguments.of("a ../ b", "a ../ b"),
27+
Arguments.of("a /.. b", "a /.. b"),
28+
Arguments.of("a //..// b", "a //..// b"),
29+
Arguments.of("a / b", "a / b"),
30+
Arguments.of("a ~/ b", "a ~/ b"),
31+
Arguments.of("a //// b", "a //// b"),
32+
Arguments.of("a .. b", "a .. b"),
33+
Arguments.of("a . b", "a . b"),
34+
Arguments.of(" lots of space ", "lots of space"),
35+
Arguments.of(
2936
"Failed to save c:/fooß/aïböcß/aób∑c/∑ö/ππ¨p/ö/a/bar123öabc/baz.txt no permissions (error!)",
30-
fakeUser
37+
"Failed to save c:/xß/xï/xó/x∑/xπ/xö/x/xö/x.txt no permissions (error!)"
38+
),
39+
Arguments.of(
40+
"user: jdoe123 file: C:/Users/user1/.aws/sso/cache/abc123.json (regex: /foo/)",
41+
"user: x file: C:/Users/x/.aws/sso/cache/x.json (regex: /x/)"
42+
),
43+
Arguments.of("/Users/user1/foo.jso", "/Users/x/x.jso"),
44+
Arguments.of("/Users/user1/foo.js", "/Users/x/x.js"),
45+
Arguments.of("/Users/user1/noFileExtension", "/Users/x/x"),
46+
Arguments.of("/Users/user1/minExtLength.a", "/Users/x/x.a"),
47+
Arguments.of("/Users/user1/extIsNum.123456", "/Users/x/x.123456"),
48+
Arguments.of("/Users/user1/foo.looooooooongextension", "/Users/x/x.looooooooongextension"),
49+
Arguments.of("/Users/user1/multipleExts.ext1.ext2.ext3", "/Users/x/x.ext3"),
50+
Arguments.of("c:\\fooß\\bar\\baz.txt", "c:/xß/x/x.txt"),
51+
Arguments.of("unc path: \\\\server$\\pipename\\etc END", "unc path: //x$/x/x END"),
52+
Arguments.of(
53+
"c:\\Users\\user1\\.aws\\sso\\cache\\abc123.json jdoe123 abc",
54+
"c:/Users/x/.aws/sso/cache/x.json x abc"
55+
),
56+
Arguments.of("unix /home/jdoe123/.aws/config failed", "unix /home/x/.aws/config failed"),
57+
Arguments.of("unix ~jdoe123/.aws/config failed", "unix ~x/.aws/config failed"),
58+
Arguments.of("unix ../../.aws/config failed", "unix ../../.aws/config failed"),
59+
Arguments.of("unix ~/.aws/config failed", "unix ~/.aws/config failed"),
60+
Arguments.of(
61+
"/Users/user1/.aws/sso/cache/abc123.json no space",
62+
"/Users/x/.aws/sso/cache/x.json no space"
3163
)
3264
)
33-
assertEquals(
34-
"user: x file: C:/Users/x/.aws/sso/cache/x.json (regex: /x/)",
35-
scrubNames("user: jdoe123 file: C:/Users/user1/.aws/sso/cache/abc123.json (regex: /foo/)", fakeUser)
36-
)
37-
assertEquals("/Users/x/x.jso", scrubNames("/Users/user1/foo.jso", fakeUser))
38-
assertEquals("/Users/x/x.js", scrubNames("/Users/user1/foo.js", fakeUser))
39-
assertEquals("/Users/x/x", scrubNames("/Users/user1/noFileExtension", fakeUser))
40-
assertEquals("/Users/x/x.a", scrubNames("/Users/user1/minExtLength.a", fakeUser))
41-
assertEquals("/Users/x/x.123456", scrubNames("/Users/user1/extIsNum.123456", fakeUser))
42-
assertEquals(
43-
"/Users/x/x.looooooooongextension",
44-
scrubNames("/Users/user1/foo.looooooooongextension", fakeUser)
45-
)
46-
assertEquals("/Users/x/x.ext3", scrubNames("/Users/user1/multipleExts.ext1.ext2.ext3", fakeUser))
47-
assertEquals("c:/xß/x/x.txt", scrubNames("c:\\fooß\\bar\\baz.txt", fakeUser))
48-
assertEquals(
49-
"unc path: //x$/x/x END",
50-
scrubNames("unc path: \\\\server$\\pipename\\etc END", fakeUser)
51-
)
52-
assertEquals(
53-
"c:/Users/x/.aws/sso/cache/x.json x abc",
54-
scrubNames("c:\\Users\\user1\\.aws\\sso\\cache\\abc123.json jdoe123 abc", fakeUser)
55-
)
56-
assertEquals(
57-
"unix /home/x/.aws/config failed",
58-
scrubNames("unix /home/jdoe123/.aws/config failed", fakeUser)
59-
)
60-
assertEquals(
61-
"unix ~x/.aws/config failed",
62-
scrubNames("unix ~jdoe123/.aws/config failed", fakeUser)
63-
)
64-
assertEquals(
65-
"unix ../../.aws/config failed",
66-
scrubNames("unix ../../.aws/config failed", fakeUser)
67-
)
68-
assertEquals("unix ~/.aws/config failed", scrubNames("unix ~/.aws/config failed", fakeUser))
69-
assertEquals(
70-
"/Users/x/.aws/sso/cache/x.json no space",
71-
scrubNames("/Users/user1/.aws/sso/cache/abc123.json no space", fakeUser)
72-
)
7365
}
7466
}

0 commit comments

Comments
 (0)