Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update description with crash #627

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ dependencies {

implementation("com.uchuhimo", "konf", "0.22.1")
implementation("com.github.rcarz", "jira-client", "master-SNAPSHOT")
implementation("me.urielsalis", "mc-crash-lib", "master-SNAPSHOT")
implementation("me.urielsalis", "mc-crash-lib", "1.0.3")
implementation("com.github.napstr", "logback-discord-appender", "1.0.0")
implementation("org.slf4j", "slf4j-api", "1.7.25")
implementation("ch.qos.logback", "logback-classic", logBackVersion)
Expand Down
13 changes: 13 additions & 0 deletions config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,19 @@ arisa:
exceptionRegex: 'ig[0-9]{1,2}icd[0-9]{2}\.dll'
duplicates: MC-32606

crashInfo:
projects:
- MC
- MCL
- MCTEST
resolutions:
- Unresolved
excludedStatuses:
- Postponed
crashExtensions:
- txt
- log

missingCrash:
enabled: false
resolutions:
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ kotlin.code.style=official
org.gradle.caching=true
org.gradle.console=auto
org.gradle.daemon=false
org.gradle.parallel=true
org.gradle.parallel=true
9 changes: 9 additions & 0 deletions src/main/kotlin/io/github/mojira/arisa/ModuleRegistry.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import io.github.mojira.arisa.modules.AttachmentModule
import io.github.mojira.arisa.modules.CHKModule
import io.github.mojira.arisa.modules.CommandModule
import io.github.mojira.arisa.modules.ConfirmParentModule
import io.github.mojira.arisa.modules.CrashInfoModule
import io.github.mojira.arisa.modules.CrashModule
import io.github.mojira.arisa.modules.DuplicateMessageModule
import io.github.mojira.arisa.modules.EmptyModule
Expand Down Expand Up @@ -148,6 +149,14 @@ class ModuleRegistry(private val config: Config) {
)
)

register(
Modules.CrashInfo,
CrashInfoModule(
config[Modules.CrashInfo.crashExtensions],
CrashReader()
)
)

register(
Modules.MissingCrash,
MissingCrashModule(
Expand Down
1 change: 1 addition & 0 deletions src/main/kotlin/io/github/mojira/arisa/domain/Issue.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ data class Issue(
val addComment: (options: CommentOptions) -> Unit,
val addRestrictedComment: (options: CommentOptions) -> Unit,
val addNotEnglishComment: (language: String) -> Unit,
val addRawComment: (body: String) -> Unit,
val addRawRestrictedComment: (body: String, restriction: String) -> Unit,
val markAsFixedWithSpecificVersion: (fixVersion: String) -> Unit,
val changeReporter: (reporter: String) -> Unit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,12 @@ object Arisa : ConfigSpec() {
)
}

object CrashInfo : ModuleConfigSpec() {
val crashExtensions by required<List<String>>(
description = "File extensions that should be checked for crash reports."
)
}

object MissingCrash : ModuleConfigSpec() {
val crashExtensions by required<List<String>>(
description = "File extensions that should be checked for crash reports."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,26 @@ fun addRestrictedComment(
}
}

fun addRawComment(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed anymore, is it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to leave it for future purposes.

context: Lazy<IssueUpdateContext>,
comment: String
) {
context.value.otherOperations.add {
runBlocking {
Either.catch {
val key = context.value.jiraIssue.key

when (val checkResult = CommentCache.check(key, comment)) {
is Either.Left -> log.error(checkResult.a.message)
is Either.Right -> context.value.jiraIssue.addComment(comment)
}

Unit
}
}
}
}

fun createLink(
context: Lazy<IssueUpdateContext>,
getContext: (key: String) -> Lazy<IssueUpdateContext>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ fun JiraIssue.toDomain(
)
)
},
addRawComment = ::addRawComment.partially1(context),
addRawRestrictedComment = ::addRestrictedComment.partially1(context),
::markAsFixedWithSpecificVersion.partially1(context),
::changeReporter.partially1(context),
Expand Down
90 changes: 90 additions & 0 deletions src/main/kotlin/io/github/mojira/arisa/modules/CrashInfoModule.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package io.github.mojira.arisa.modules

import arrow.core.Either
import arrow.core.extensions.fx
import arrow.core.left
import arrow.core.right
import arrow.syntax.function.partially2
import io.github.mojira.arisa.domain.Issue
import io.github.mojira.arisa.infrastructure.AttachmentUtils
import me.urielsalis.mccrashlib.Crash
import me.urielsalis.mccrashlib.CrashReader
import java.io.File
import java.nio.file.Files
import java.time.Instant

val crashesDir = Files.createTempDirectory("crashes").toFile()

class CrashInfoModule(
private val crashReportExtensions: List<String>,
private val crashReader: CrashReader
) : Module {
override fun invoke(issue: Issue, lastRun: Instant): Either<ModuleError, ModuleResponse> = with(issue) {
Either.fx {
val crashes = AttachmentUtils(crashReportExtensions, crashReader).extractCrashesFromAttachments(issue)
val newCrashes = assertContainsNewCrash(crashes, lastRun).bind()
uploadDeobfuscatedCrashes(issue, newCrashes)

newCrashes
.filter { it.second is Crash.Minecraft }
.filterNot { it.first.name.endsWith("deobfuscated.txt") }
Comment on lines +26 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably restrict this to crash reports attached by the reporter (or by a helper / moderator?).
Otherwise users can vandalize existing issues by attaching unrelated crash reports, or files which Arisa considers to be crash reports.

.forEach {
addRawComment(generateCrashMessage(it.first.name, it.second as Crash.Minecraft))
}
}
}

private fun generateCrashMessage(name: String, minecraft: Crash.Minecraft) =
"{code:title=(${minecraft.minecraftVersion}) [^$name]}\n\n" +
"Description: ${minecraft.exception.split("\n").first()}\n\n" +
"Exception: ${minecraft.exception}\n\n" +
(if (minecraft.deobfException != null) "Deobfuscated: ${minecraft.deobfException}\n" else "\n") +
"{code}\n"

private fun uploadDeobfuscatedCrashes(issue: Issue, crashes: List<Pair<AttachmentUtils.TextDocument, Crash>>) {
val minecraftCrashesWithDeobf = crashes
.map { it.first.name to it.second }
.filter { it.second is Crash.Minecraft }
.map { it.first to (it.second as Crash.Minecraft).deobf }
.filter { it.second != null }
.filterNot {
issue.attachments.any { attachment ->
attachment.name == getDeobfName(it.first) || attachment.name.endsWith(
"deobfuscated.txt"
)
}
}
minecraftCrashesWithDeobf.forEach {
if (isNotZipSlip(getDeobfName(it.first))) {
val file = File(crashesDir, getDeobfName(it.first))
file.writeText(it.second!!)
issue.addAttachment(file)
urielsalis marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

private fun isNotZipSlip(version: String): Boolean {
val canonicalDestinationDir = crashesDir.canonicalPath
val destinationFile = File(crashesDir, version)
val canonicalDestinationFile = destinationFile.canonicalPath
return canonicalDestinationFile.startsWith(canonicalDestinationDir + File.separator)
}

private fun getDeobfName(name: String): String =
"${name.substringBeforeLast(".").replace("\\", "").replace("/", "")}-deobfuscated.txt"

private fun crashNewlyAdded(crash: Pair<AttachmentUtils.TextDocument, Crash>, lastRun: Instant) =
crash.first.created.isAfter(lastRun)

private fun assertContainsNewCrash(
crashes: List<Pair<AttachmentUtils.TextDocument, Crash>>,
lastRun: Instant
): Either<OperationNotNeededModuleResponse, List<Pair<AttachmentUtils.TextDocument, Crash>>> {
val newCrashes = crashes.filter(::crashNewlyAdded.partially2(lastRun))
return if (newCrashes.isNotEmpty()) {
newCrashes.right()
} else {
OperationNotNeededModuleResponse.left()
}
}
}
26 changes: 1 addition & 25 deletions src/main/kotlin/io/github/mojira/arisa/modules/CrashModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import io.github.mojira.arisa.infrastructure.AttachmentUtils
import io.github.mojira.arisa.infrastructure.config.CrashDupeConfig
import me.urielsalis.mccrashlib.Crash
import me.urielsalis.mccrashlib.CrashReader
import java.io.File
import java.time.Instant

class CrashModule(
Expand All @@ -29,8 +28,7 @@ class CrashModule(

val crashes = AttachmentUtils(crashReportExtensions, crashReader).extractCrashesFromAttachments(issue)

val newCrashes = assertContainsNewCrash(crashes, lastRun).bind()
uploadDeobfuscatedCrashes(issue, newCrashes)
assertContainsNewCrash(crashes, lastRun).bind()
assertNoValidCrash(crashes).bind()

val key = crashes
Expand All @@ -49,28 +47,6 @@ class CrashModule(
}
}

private fun uploadDeobfuscatedCrashes(issue: Issue, crashes: List<Pair<AttachmentUtils.TextDocument, Crash>>) {
val minecraftCrashesWithDeobf = crashes
.map { it.first.name to it.second }
.filter { it.second is Crash.Minecraft }
.map { it.first to (it.second as Crash.Minecraft).deobf }
.filter { it.second != null }
.filterNot {
issue.attachments.any { attachment ->
attachment.name == getDeobfName(it.first) || attachment.name.endsWith(
"deobfuscated.txt"
)
}
}
minecraftCrashesWithDeobf.forEach {
val file = File(getDeobfName(it.first))
file.writeText(it.second!!)
issue.addAttachment(file)
}
}

private fun getDeobfName(name: String): String = "${name.substringBeforeLast(".")}-deobfuscated.txt"

/**
* Checks whether an analyzed crash report matches any of the specified known crash issues.
* Returns the key of the parent bug report if one is found, and null otherwise.
Expand Down
Loading