Improve DavException construction

build.gradle.kts

@@ -55,6 +55,7 @@ tasks.withType<DokkaTask>().configureEach {
 
 dependencies {
     api(libs.okhttp)
+    api(libs.spotbugs.annotations)
     api(libs.xpp3)
 
     testImplementation(libs.junit4)

gradle/libs.versions.toml

@@ -3,12 +3,14 @@ dokka = "2.0.0"
 junit4 = "4.13.2"
 kotlin = "2.2.0"
 okhttpVersion = "5.1.0"
+spotbugs = "4.9.4"
 xpp3Version = "1.1.6"
 
 [libraries]
 junit4 = { module = "junit:junit", version.ref = "junit4" }
 okhttp = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttpVersion" }
 okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver3", version.ref = "okhttpVersion" }
+spotbugs-annotations = { module = "com.github.spotbugs:spotbugs-annotations", version.ref = "spotbugs" }
 xpp3 = { module = "org.ogce:xpp3", version.ref = "xpp3Version" }
 
 [plugins]

src/main/kotlin/at/bitfire/dav4jvm/DavResource.kt

@@ -16,6 +16,7 @@ import at.bitfire.dav4jvm.XmlUtils.propertyName
 import at.bitfire.dav4jvm.exception.ConflictException
 import at.bitfire.dav4jvm.exception.DavException
 import at.bitfire.dav4jvm.exception.ForbiddenException
+import at.bitfire.dav4jvm.exception.GoneException
 import at.bitfire.dav4jvm.exception.HttpException
 import at.bitfire.dav4jvm.exception.NotFoundException
 import at.bitfire.dav4jvm.exception.PreconditionFailedException
@@ -39,7 +40,6 @@ import java.io.EOFException
 import java.io.IOException
 import java.io.Reader
 import java.io.StringWriter
-import java.net.HttpURLConnection
 import java.util.logging.Level
 import java.util.logging.Logger
 import at.bitfire.dav4jvm.Response as DavResponse
@@ -666,34 +666,20 @@ open class DavResource @JvmOverloads constructor(
      *
      * @throws HttpException in case of an HTTP error
      */
-    protected fun checkStatus(response: Response) =
-            checkStatus(response.code, response.message, response)
-
-    /**
-     * Checks the status from an HTTP response and throws an exception in case of an error.
-     *
-     * @throws HttpException (with XML error names, if available) in case of an HTTP error
-     */
-    private fun checkStatus(code: Int, message: String?, response: Response?) {
-        if (code / 100 == 2)
+    protected fun checkStatus(response: Response) {
+        if (response.code / 100 == 2)
             // everything OK
             return
 
-        throw when (code) {
-            HttpURLConnection.HTTP_UNAUTHORIZED ->
-                if (response != null) UnauthorizedException(response) else UnauthorizedException(message)
-            HttpURLConnection.HTTP_FORBIDDEN ->
-                if (response != null) ForbiddenException(response) else ForbiddenException(message)
-            HttpURLConnection.HTTP_NOT_FOUND ->
-                if (response != null) NotFoundException(response) else NotFoundException(message)
-            HttpURLConnection.HTTP_CONFLICT ->
-                if (response != null) ConflictException(response) else ConflictException(message)
-            HttpURLConnection.HTTP_PRECON_FAILED ->
-                if (response != null) PreconditionFailedException(response) else PreconditionFailedException(message)
-            HttpURLConnection.HTTP_UNAVAILABLE ->
-                if (response != null) ServiceUnavailableException(response) else ServiceUnavailableException(message)
-            else ->
-                if (response != null) HttpException(response) else HttpException(code, message)
+        throw when (response.code) {
+            401 -> UnauthorizedException(response)
+            403 -> ForbiddenException(response)
+            404 -> NotFoundException(response)
+            409 -> ConflictException(response)
+            410 -> GoneException(response)
+            412 -> PreconditionFailedException(response)
+            503 -> ServiceUnavailableException(response)
+            else -> HttpException(response)
         }
     }
 
@@ -739,7 +725,7 @@ open class DavResource @JvmOverloads constructor(
      */
     fun assertMultiStatus(response: Response) {
         if (response.code != HTTP_MULTISTATUS)
-            throw DavException("Expected 207 Multi-Status, got ${response.code} ${response.message}", httpResponse = response)
+            throw DavException("Expected 207 Multi-Status, got ${response.code} ${response.message}", response = response)
 
         response.peekBody(XML_SIGNATURE.size.toLong()).use { body ->
             body.contentType()?.let { mimeType ->
@@ -760,7 +746,7 @@ open class DavResource @JvmOverloads constructor(
                         logger.log(Level.WARNING, "Couldn't scan for XML signature", e)
                     }
 
-                    throw DavException("Received non-XML 207 Multi-Status", httpResponse = response)
+                    throw DavException("Received non-XML 207 Multi-Status", response = response)
                 }
             } ?: logger.warning("Received 207 Multi-Status without Content-Type, assuming XML")
         }

src/main/kotlin/at/bitfire/dav4jvm/Error.kt

@@ -20,8 +20,8 @@ import java.io.Serializable
  *
  * At the moment, there is no logic for subclassing errors.
  */
-class Error(
-        val name: Property.Name
+data class Error(
+    val name: Property.Name
 ): Serializable {
 
     companion object {

src/main/kotlin/at/bitfire/dav4jvm/exception/ConflictException.kt

@@ -11,11 +11,12 @@
 package at.bitfire.dav4jvm.exception
 
 import okhttp3.Response
-import java.net.HttpURLConnection
 
 class ConflictException: HttpException {
 
-    constructor(response: Response): super(response)
-    constructor(message: String?): super(HttpURLConnection.HTTP_CONFLICT, message)
+    constructor(response: Response) : super(response) {
+        if (response.code != 409)
+            throw IllegalArgumentException()
+    }
 
-}
+}

src/main/kotlin/at/bitfire/dav4jvm/exception/DavException.kt

@@ -13,140 +13,136 @@ package at.bitfire.dav4jvm.exception
 import at.bitfire.dav4jvm.Error
 import at.bitfire.dav4jvm.XmlUtils
 import at.bitfire.dav4jvm.XmlUtils.propertyName
-import at.bitfire.dav4jvm.exception.DavException.Companion.MAX_EXCERPT_SIZE
 import okhttp3.MediaType
 import okhttp3.Response
 import okio.Buffer
 import org.xmlpull.v1.XmlPullParser
 import org.xmlpull.v1.XmlPullParserException
 import java.io.ByteArrayOutputStream
-import java.io.IOException
-import java.io.Serializable
-import java.lang.Long.min
-import java.util.logging.Level
-import java.util.logging.Logger
+import java.io.StringReader
+import javax.annotation.WillNotClose
 
 /**
  * Signals that an error occurred during a WebDAV-related operation.
  *
- * This could be a logical error like when a required ETag has not been
- * received, but also an explicit HTTP error.
+ * This could be a logical error like when a required ETag has not been received, but also an explicit HTTP error
+ * (usually with a subclass of [HttpException], which in turn extends this class).
+ *
+ * Often, HTTP response bodies contain valuable information about the error in text format (for instance, a HTML page
+ * that contains details about the error) and/or as `<DAV:error>` XML elements. However, such response bodies
+ * are sometimes very large.
+ *
+ * So, if possible and useful, a size-limited excerpt of the associated HTTP request and response can be
+ * attached and subsequently included in application-level debug info or shown to the user.
+ *
+ * @param statusCode        status code of associated HTTP response
+ * @param requestExcerpt    cached excerpt of associated HTTP request body
+ * @param responseExcerpt   cached excerpt of associated HTTP response body
+ * @param errors            precondition/postcondition XML elements which have been found in the XML response
  */
 open class DavException @JvmOverloads constructor(
-        message: String,
-        ex: Throwable? = null,
-
-        /**
-         * An associated HTTP [Response]. Will be closed after evaluation.
-         */
-        httpResponse: Response? = null
-): Exception(message, ex), Serializable {
-
-    companion object {
-
-        const val MAX_EXCERPT_SIZE = 10*1024   // don't dump more than 20 kB
-
-        fun isPlainText(type: MediaType) =
-                type.type == "text" ||
-                (type.type == "application" && type.subtype in arrayOf("html", "xml"))
-
-    }
+    message: String? = null,
+    cause: Throwable? = null,
+    statusCode: Int? = null,
+    requestExcerpt: String? = null,
+    responseExcerpt: String? = null,
+    errors: List<Error> = emptyList()
+): Exception(message, cause) {
+
+    var statusCode: Int? = statusCode
+        private set
 
-    private val logger
-        get() = Logger.getLogger(javaClass.name)
+    var requestExcerpt: String? = requestExcerpt
+        private set
 
-    var request: String? = null
+    var responseExcerpt: String? = responseExcerpt
+        private set
 
-    /**
-     * Body excerpt of [request] (up to [MAX_EXCERPT_SIZE] characters). Only available
-     * if the HTTP request body was textual content and could be read again.
-     */
-    var requestBody: String? = null
+    var errors: List<Error> = errors
         private set
 
-    val response: String?
 
     /**
-     * Body excerpt of [response] (up to [MAX_EXCERPT_SIZE] characters). Only available
-     * if the HTTP response body was textual content.
+     * Takes the request, response and errors from a given HTTP response.
+     *
+     * @param response  response to extract status code and request/response excerpt from (if possible)
+     * @param message   optional exception message
+     * @param cause     optional exception cause
      */
-    var responseBody: String? = null
-        private set
+    constructor(
+        message: String?,
+        @WillNotClose response: Response,
+        cause: Throwable? = null
+    ) : this(message, cause) {
+        // extract status code
+        statusCode = response.code
+
+        // extract request body
+        val request = response.request
+        request.body?.let { requestBody ->
+            // Unfortunately doesn't have a size limit.
+            // However large bodies are usually streaming/one-shot away.
+            val buffer = Buffer()
+            requestBody.writeTo(buffer)
+
+            val baos = ByteArrayOutputStream()
+            buffer.writeTo(baos)
+            requestExcerpt = "${request.method} ${request.url}\n\n$baos"
+        }
+
+        // extract response body if response is plain text
+        val mimeType = response.body.contentType()
+        val responseBody =
+            if (mimeType?.isPlainText() == true)
+                try {
+                    response.peekBody(MAX_EXCERPT_SIZE.toLong()).string()
+                } catch (_: Exception) {
+                    // response body not available anymore, probably already consumed / closed
+                    null
+                }
+            else
+                null
+        responseExcerpt = responseBody
 
-    /**
-     * Precondition/postcondition XML elements which have been found in the XML response.
-     */
-    var errors: List<Error> = listOf()
-        private set
+        // get XML errors from request body excerpt
+        if (mimeType?.isXml() == true && responseBody != null)
+            errors = extractErrors(responseBody)
+    }
+
+    private fun extractErrors(xml: String): List<Error> {
+        try {
+            val parser = XmlUtils.newPullParser()
+            parser.setInput(StringReader(xml))
+
+            var eventType = parser.eventType
+            while (eventType != XmlPullParser.END_DOCUMENT) {
+                if (eventType == XmlPullParser.START_TAG && parser.depth == 1)
+                    if (parser.propertyName() == Error.NAME)
+                        return Error.parseError(parser)
+                eventType = parser.next()
+            }
+        } catch (_: XmlPullParserException) {
+            // Couldn't parse XML, either invalid or maybe it wasn't even XML
+        }
 
+        return emptyList()
+    }
 
-    init {
-        if (httpResponse != null) {
-            response = httpResponse.toString()
 
-            try {
-                request = httpResponse.request.toString()
+    companion object {
 
-                httpResponse.request.body?.let { body ->
-                    body.contentType()?.let { type ->
-                        if (isPlainText(type)) {
-                            val buffer = Buffer()
-                            body.writeTo(buffer)
+        /**
+         * maximum size of extracted response body
+         */
+        const val MAX_EXCERPT_SIZE = 20*1024
 
-                            val baos = ByteArrayOutputStream()
-                            buffer.writeTo(baos, min(buffer.size, MAX_EXCERPT_SIZE.toLong()))
+        private fun MediaType.isPlainText() =
+            type == "text" ||
+            (type == "application" && subtype in arrayOf("html", "xml"))
 
-                            requestBody = baos.toString(type.charset(Charsets.UTF_8)!!.name())
-                        }
-                    }
-                }
-            } catch (e: Exception) {
-                logger.log(Level.WARNING, "Couldn't read HTTP request", e)
-                requestBody = "Couldn't read HTTP request: ${e.message}"
-            }
+        private fun MediaType.isXml() =
+            type in arrayOf("application", "text") && subtype == "xml"
 
-            try {
-                // save response body excerpt
-                if (httpResponse.body?.source() != null) {
-                    // response body has a source
-
-                    httpResponse.peekBody(MAX_EXCERPT_SIZE.toLong()).let { body ->
-                        body.contentType()?.let { mimeType ->
-                            if (isPlainText(mimeType))
-                                responseBody = body.string()
-                        }
-                    }
-
-                    httpResponse.body?.use { body ->
-                        body.contentType()?.let {
-                            if (it.type in arrayOf("application", "text") && it.subtype == "xml") {
-                                // look for precondition/postcondition XML elements
-                                try {
-                                    val parser = XmlUtils.newPullParser()
-                                    parser.setInput(body.charStream())
-
-                                    var eventType = parser.eventType
-                                    while (eventType != XmlPullParser.END_DOCUMENT) {
-                                        if (eventType == XmlPullParser.START_TAG && parser.depth == 1)
-                                            if (parser.propertyName() == Error.NAME)
-                                                errors = Error.parseError(parser)
-                                        eventType = parser.next()
-                                    }
-                                } catch (e: XmlPullParserException) {
-                                    logger.log(Level.WARNING, "Couldn't parse XML response", e)
-                                }
-                            }
-                        }
-                    }
-                }
-            } catch (e: IOException) {
-                logger.log(Level.WARNING, "Couldn't read HTTP response", e)
-                responseBody = "Couldn't read HTTP response: ${e.message}"
-            } finally {
-                httpResponse.body?.close()
-            }
-        } else
-            response = null
     }
 
 }

src/main/kotlin/at/bitfire/dav4jvm/exception/ForbiddenException.kt

@@ -15,7 +15,9 @@ import java.net.HttpURLConnection
 
 class ForbiddenException: HttpException {
 
-    constructor(response: Response): super(response)
-    constructor(message: String?): super(HttpURLConnection.HTTP_FORBIDDEN, message)
+    constructor(response: Response) : super(response) {
+        if (response.code != 403)
+            throw IllegalArgumentException()
+    }
 
-}
+}