Skip to content

Commit d3e5e2c

Browse files
authored
Improve DavException construction (#83)
* Improve exception construction * Test Java serialization support of DavException * Request excerpt: only use for plain text, truncate * Improve exception construction messages
1 parent 02fe1a9 commit d3e5e2c

17 files changed

+350
-357
lines changed

build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ tasks.withType<DokkaTask>().configureEach {
5555

5656
dependencies {
5757
api(libs.okhttp)
58+
api(libs.spotbugs.annotations)
5859
api(libs.xpp3)
5960

6061
testImplementation(libs.junit4)

gradle/libs.versions.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ dokka = "2.0.0"
33
junit4 = "4.13.2"
44
kotlin = "2.2.0"
55
okhttpVersion = "5.1.0"
6+
spotbugs = "4.9.4"
67
xpp3Version = "1.1.6"
78

89
[libraries]
910
junit4 = { module = "junit:junit", version.ref = "junit4" }
1011
okhttp = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttpVersion" }
1112
okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver3", version.ref = "okhttpVersion" }
13+
spotbugs-annotations = { module = "com.github.spotbugs:spotbugs-annotations", version.ref = "spotbugs" }
1214
xpp3 = { module = "org.ogce:xpp3", version.ref = "xpp3Version" }
1315

1416
[plugins]

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

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import at.bitfire.dav4jvm.XmlUtils.propertyName
1616
import at.bitfire.dav4jvm.exception.ConflictException
1717
import at.bitfire.dav4jvm.exception.DavException
1818
import at.bitfire.dav4jvm.exception.ForbiddenException
19+
import at.bitfire.dav4jvm.exception.GoneException
1920
import at.bitfire.dav4jvm.exception.HttpException
2021
import at.bitfire.dav4jvm.exception.NotFoundException
2122
import at.bitfire.dav4jvm.exception.PreconditionFailedException
@@ -39,7 +40,6 @@ import java.io.EOFException
3940
import java.io.IOException
4041
import java.io.Reader
4142
import java.io.StringWriter
42-
import java.net.HttpURLConnection
4343
import java.util.logging.Level
4444
import java.util.logging.Logger
4545
import at.bitfire.dav4jvm.Response as DavResponse
@@ -666,34 +666,20 @@ open class DavResource @JvmOverloads constructor(
666666
*
667667
* @throws HttpException in case of an HTTP error
668668
*/
669-
protected fun checkStatus(response: Response) =
670-
checkStatus(response.code, response.message, response)
671-
672-
/**
673-
* Checks the status from an HTTP response and throws an exception in case of an error.
674-
*
675-
* @throws HttpException (with XML error names, if available) in case of an HTTP error
676-
*/
677-
private fun checkStatus(code: Int, message: String?, response: Response?) {
678-
if (code / 100 == 2)
669+
protected fun checkStatus(response: Response) {
670+
if (response.code / 100 == 2)
679671
// everything OK
680672
return
681673

682-
throw when (code) {
683-
HttpURLConnection.HTTP_UNAUTHORIZED ->
684-
if (response != null) UnauthorizedException(response) else UnauthorizedException(message)
685-
HttpURLConnection.HTTP_FORBIDDEN ->
686-
if (response != null) ForbiddenException(response) else ForbiddenException(message)
687-
HttpURLConnection.HTTP_NOT_FOUND ->
688-
if (response != null) NotFoundException(response) else NotFoundException(message)
689-
HttpURLConnection.HTTP_CONFLICT ->
690-
if (response != null) ConflictException(response) else ConflictException(message)
691-
HttpURLConnection.HTTP_PRECON_FAILED ->
692-
if (response != null) PreconditionFailedException(response) else PreconditionFailedException(message)
693-
HttpURLConnection.HTTP_UNAVAILABLE ->
694-
if (response != null) ServiceUnavailableException(response) else ServiceUnavailableException(message)
695-
else ->
696-
if (response != null) HttpException(response) else HttpException(code, message)
674+
throw when (response.code) {
675+
401 -> UnauthorizedException(response)
676+
403 -> ForbiddenException(response)
677+
404 -> NotFoundException(response)
678+
409 -> ConflictException(response)
679+
410 -> GoneException(response)
680+
412 -> PreconditionFailedException(response)
681+
503 -> ServiceUnavailableException(response)
682+
else -> HttpException(response)
697683
}
698684
}
699685

@@ -739,7 +725,7 @@ open class DavResource @JvmOverloads constructor(
739725
*/
740726
fun assertMultiStatus(response: Response) {
741727
if (response.code != HTTP_MULTISTATUS)
742-
throw DavException("Expected 207 Multi-Status, got ${response.code} ${response.message}", httpResponse = response)
728+
throw DavException("Expected 207 Multi-Status, got ${response.code} ${response.message}", response = response)
743729

744730
response.peekBody(XML_SIGNATURE.size.toLong()).use { body ->
745731
body.contentType()?.let { mimeType ->
@@ -760,7 +746,7 @@ open class DavResource @JvmOverloads constructor(
760746
logger.log(Level.WARNING, "Couldn't scan for XML signature", e)
761747
}
762748

763-
throw DavException("Received non-XML 207 Multi-Status", httpResponse = response)
749+
throw DavException("Received non-XML 207 Multi-Status", response = response)
764750
}
765751
} ?: logger.warning("Received 207 Multi-Status without Content-Type, assuming XML")
766752
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ import java.io.Serializable
1919
* name. Subclassed errors may have more specific information available.
2020
*
2121
* At the moment, there is no logic for subclassing errors.
22+
*
23+
* @param name property name for the XML error element
2224
*/
23-
class Error(
24-
val name: Property.Name
25+
data class Error(
26+
val name: Property.Name
2527
): Serializable {
2628

2729
companion object {

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@
1111
package at.bitfire.dav4jvm.exception
1212

1313
import okhttp3.Response
14-
import java.net.HttpURLConnection
1514

1615
class ConflictException: HttpException {
1716

18-
constructor(response: Response): super(response)
19-
constructor(message: String?): super(HttpURLConnection.HTTP_CONFLICT, message)
17+
constructor(response: Response) : super(response) {
18+
if (response.code != 409)
19+
throw IllegalArgumentException("Status code must be 409")
20+
}
2021

21-
}
22+
}

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

Lines changed: 115 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -13,140 +13,148 @@ package at.bitfire.dav4jvm.exception
1313
import at.bitfire.dav4jvm.Error
1414
import at.bitfire.dav4jvm.XmlUtils
1515
import at.bitfire.dav4jvm.XmlUtils.propertyName
16-
import at.bitfire.dav4jvm.exception.DavException.Companion.MAX_EXCERPT_SIZE
1716
import okhttp3.MediaType
1817
import okhttp3.Response
1918
import okio.Buffer
2019
import org.xmlpull.v1.XmlPullParser
2120
import org.xmlpull.v1.XmlPullParserException
2221
import java.io.ByteArrayOutputStream
23-
import java.io.IOException
24-
import java.io.Serializable
25-
import java.lang.Long.min
26-
import java.util.logging.Level
27-
import java.util.logging.Logger
22+
import java.io.StringReader
23+
import javax.annotation.WillNotClose
24+
import kotlin.math.min
2825

2926
/**
3027
* Signals that an error occurred during a WebDAV-related operation.
3128
*
32-
* This could be a logical error like when a required ETag has not been
33-
* received, but also an explicit HTTP error.
29+
* This could be a logical error like when a required ETag has not been received, but also an explicit HTTP error
30+
* (usually with a subclass of [HttpException], which in turn extends this class).
31+
*
32+
* Often, HTTP response bodies contain valuable information about the error in text format (for instance, a HTML page
33+
* that contains details about the error) and/or as `<DAV:error>` XML elements. However, such response bodies
34+
* are sometimes very large.
35+
*
36+
* So, if possible and useful, a size-limited excerpt of the associated HTTP request and response can be
37+
* attached and subsequently included in application-level debug info or shown to the user.
38+
*
39+
* Note: [Exception] is serializable, so objects of this class must contain only serializable objects.
40+
*
41+
* @param statusCode status code of associated HTTP response
42+
* @param requestExcerpt cached excerpt of associated HTTP request body
43+
* @param responseExcerpt cached excerpt of associated HTTP response body
44+
* @param errors precondition/postcondition XML elements which have been found in the XML response
3445
*/
3546
open class DavException @JvmOverloads constructor(
36-
message: String,
37-
ex: Throwable? = null,
38-
39-
/**
40-
* An associated HTTP [Response]. Will be closed after evaluation.
41-
*/
42-
httpResponse: Response? = null
43-
): Exception(message, ex), Serializable {
44-
45-
companion object {
46-
47-
const val MAX_EXCERPT_SIZE = 10*1024 // don't dump more than 20 kB
48-
49-
fun isPlainText(type: MediaType) =
50-
type.type == "text" ||
51-
(type.type == "application" && type.subtype in arrayOf("html", "xml"))
52-
53-
}
54-
55-
private val logger
56-
get() = Logger.getLogger(javaClass.name)
57-
58-
var request: String? = null
47+
message: String? = null,
48+
cause: Throwable? = null,
49+
statusCode: Int? = null,
50+
requestExcerpt: String? = null,
51+
responseExcerpt: String? = null,
52+
errors: List<Error> = emptyList()
53+
): Exception(message, cause) {
54+
55+
var statusCode: Int? = statusCode
56+
private set
5957

60-
/**
61-
* Body excerpt of [request] (up to [MAX_EXCERPT_SIZE] characters). Only available
62-
* if the HTTP request body was textual content and could be read again.
63-
*/
64-
var requestBody: String? = null
58+
var requestExcerpt: String? = requestExcerpt
6559
private set
6660

67-
val response: String?
61+
var responseExcerpt: String? = responseExcerpt
62+
private set
6863

69-
/**
70-
* Body excerpt of [response] (up to [MAX_EXCERPT_SIZE] characters). Only available
71-
* if the HTTP response body was textual content.
72-
*/
73-
var responseBody: String? = null
64+
var errors: List<Error> = errors
7465
private set
7566

7667
/**
77-
* Precondition/postcondition XML elements which have been found in the XML response.
68+
* Takes the request, response and errors from a given HTTP response.
69+
*
70+
* @param response response to extract status code and request/response excerpt from (if possible)
71+
* @param message optional exception message
72+
* @param cause optional exception cause
7873
*/
79-
var errors: List<Error> = listOf()
80-
private set
74+
constructor(
75+
message: String?,
76+
@WillNotClose response: Response,
77+
cause: Throwable? = null
78+
) : this(message, cause) {
79+
// extract status code
80+
statusCode = response.code
81+
82+
// extract request body if it's text
83+
val request = response.request
84+
val requestExcerptBuilder = StringBuilder(
85+
"${request.method} ${request.url}"
86+
)
87+
request.body?.let { requestBody ->
88+
if (requestBody.contentType()?.isText() == true) {
89+
// Unfortunately Buffer doesn't have a size limit.
90+
// However large bodies are usually streaming/one-shot away.
91+
val buffer = Buffer()
92+
requestBody.writeTo(buffer)
93+
94+
ByteArrayOutputStream().use { baos ->
95+
buffer.writeTo(baos, min(buffer.size, MAX_EXCERPT_SIZE.toLong()))
96+
requestExcerptBuilder
97+
.append("\n\n")
98+
.append(baos.toString())
99+
}
100+
} else
101+
requestExcerptBuilder.append("\n\n<request body>")
102+
}
103+
requestExcerpt = requestExcerptBuilder.toString()
104+
105+
// extract response body if it's text
106+
val mimeType = response.body.contentType()
107+
val responseBody =
108+
if (mimeType?.isText() == true)
109+
try {
110+
response.peekBody(MAX_EXCERPT_SIZE.toLong()).string()
111+
} catch (_: Exception) {
112+
// response body not available anymore, probably already consumed / closed
113+
null
114+
}
115+
else
116+
null
117+
responseExcerpt = responseBody
118+
119+
// get XML errors from request body excerpt
120+
if (mimeType?.isXml() == true && responseBody != null)
121+
errors = extractErrors(responseBody)
122+
}
123+
124+
private fun extractErrors(xml: String): List<Error> {
125+
try {
126+
val parser = XmlUtils.newPullParser()
127+
parser.setInput(StringReader(xml))
128+
129+
var eventType = parser.eventType
130+
while (eventType != XmlPullParser.END_DOCUMENT) {
131+
if (eventType == XmlPullParser.START_TAG && parser.depth == 1)
132+
if (parser.propertyName() == Error.NAME)
133+
return Error.parseError(parser)
134+
eventType = parser.next()
135+
}
136+
} catch (_: XmlPullParserException) {
137+
// Couldn't parse XML, either invalid or maybe it wasn't even XML
138+
}
81139

140+
return emptyList()
141+
}
82142

83-
init {
84-
if (httpResponse != null) {
85-
response = httpResponse.toString()
86143

87-
try {
88-
request = httpResponse.request.toString()
144+
companion object {
89145

90-
httpResponse.request.body?.let { body ->
91-
body.contentType()?.let { type ->
92-
if (isPlainText(type)) {
93-
val buffer = Buffer()
94-
body.writeTo(buffer)
146+
/**
147+
* maximum size of extracted response body
148+
*/
149+
const val MAX_EXCERPT_SIZE = 20*1024
95150

96-
val baos = ByteArrayOutputStream()
97-
buffer.writeTo(baos, min(buffer.size, MAX_EXCERPT_SIZE.toLong()))
151+
private fun MediaType.isText() =
152+
type == "text" ||
153+
(type == "application" && subtype in arrayOf("html", "xml"))
98154

99-
requestBody = baos.toString(type.charset(Charsets.UTF_8)!!.name())
100-
}
101-
}
102-
}
103-
} catch (e: Exception) {
104-
logger.log(Level.WARNING, "Couldn't read HTTP request", e)
105-
requestBody = "Couldn't read HTTP request: ${e.message}"
106-
}
155+
private fun MediaType.isXml() =
156+
type in arrayOf("application", "text") && subtype == "xml"
107157

108-
try {
109-
// save response body excerpt
110-
if (httpResponse.body?.source() != null) {
111-
// response body has a source
112-
113-
httpResponse.peekBody(MAX_EXCERPT_SIZE.toLong()).let { body ->
114-
body.contentType()?.let { mimeType ->
115-
if (isPlainText(mimeType))
116-
responseBody = body.string()
117-
}
118-
}
119-
120-
httpResponse.body?.use { body ->
121-
body.contentType()?.let {
122-
if (it.type in arrayOf("application", "text") && it.subtype == "xml") {
123-
// look for precondition/postcondition XML elements
124-
try {
125-
val parser = XmlUtils.newPullParser()
126-
parser.setInput(body.charStream())
127-
128-
var eventType = parser.eventType
129-
while (eventType != XmlPullParser.END_DOCUMENT) {
130-
if (eventType == XmlPullParser.START_TAG && parser.depth == 1)
131-
if (parser.propertyName() == Error.NAME)
132-
errors = Error.parseError(parser)
133-
eventType = parser.next()
134-
}
135-
} catch (e: XmlPullParserException) {
136-
logger.log(Level.WARNING, "Couldn't parse XML response", e)
137-
}
138-
}
139-
}
140-
}
141-
}
142-
} catch (e: IOException) {
143-
logger.log(Level.WARNING, "Couldn't read HTTP response", e)
144-
responseBody = "Couldn't read HTTP response: ${e.message}"
145-
} finally {
146-
httpResponse.body?.close()
147-
}
148-
} else
149-
response = null
150158
}
151159

152160
}

0 commit comments

Comments
 (0)