diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index b2f183eb..2e921232 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -2,13 +2,5 @@ \ No newline at end of file diff --git a/src/main/kotlin/at/bitfire/dav4jvm/ktor/DavResource.kt b/src/main/kotlin/at/bitfire/dav4jvm/ktor/DavResource.kt index d2861366..14e563b8 100644 --- a/src/main/kotlin/at/bitfire/dav4jvm/ktor/DavResource.kt +++ b/src/main/kotlin/at/bitfire/dav4jvm/ktor/DavResource.kt @@ -220,14 +220,7 @@ open class DavResource( header(HttpHeaders.Overwrite, "F") } }) { response -> - checkStatus(response) - - if (response.status == HttpStatusCode.MultiStatus) { - /* Multiple resources were to be affected by the MOVE, but errors on some - of them prevented the operation from taking place. - [_] (RFC 4918 9.9.4. Status Codes for MOVE Method) */ - throw HttpException.fromResponse(response) - } + checkStatus(response, multiStatusIsError = true) // update location val nPath = response.headers[HttpHeaders.Location] ?: destination.toString() @@ -258,13 +251,7 @@ open class DavResource( header("Overwrite", "F") } }) { response -> - checkStatus(response) - - if (response.status == HttpStatusCode.MultiStatus) - /* Multiple resources were to be affected by the COPY, but errors on some - of them prevented the operation from taking place. - [_] (RFC 4918 9.8.5. Status Codes for COPY Method) */ - throw HttpException.fromResponse(response) + checkStatus(response, multiStatusIsError = true) callback.onResponse(response) } @@ -301,7 +288,7 @@ open class DavResource( setBody(xmlBody) } }) { response -> - checkStatus(response) + checkStatus(response, multiStatusIsError = true) callback.onResponse(response) } } @@ -470,13 +457,7 @@ open class DavResource( headers.appendAll(additionalHeaders) } }) { response -> - checkStatus(response) - - if (response.status == HttpStatusCode.MultiStatus) - /* If an error occurs deleting a member resource (a resource other than - the resource identified in the Request-URI), then the response can be - a 207 (Multi-Status). […] (RFC 4918 9.6.1. DELETE for Collections) */ - throw HttpException.fromResponse(response) + checkStatus(response, multiStatusIsError = true) callback.onResponse(response) } @@ -597,12 +578,18 @@ open class DavResource( /** * Checks the status from an HTTP response and throws a specific exception in case of an error. * + * @param multiStatusIsError when true, 207 Multi-Status is treated as an error * @throws HttpException in case of an HTTP error */ - protected suspend fun checkStatus(response: HttpResponse) { - if (response.status.isSuccess()) - return // everything OK + protected suspend fun checkStatus(response: HttpResponse, multiStatusIsError: Boolean = false) { + // handle 2xx response codes + if (response.status.isSuccess()) { + if (response.status == HttpStatusCode.MultiStatus && multiStatusIsError) + throw HttpException.fromResponse(response) + return + } + // handle other response codes throw HttpException.fromResponse(response) } diff --git a/src/main/kotlin/at/bitfire/dav4jvm/okhttp/DavResource.kt b/src/main/kotlin/at/bitfire/dav4jvm/okhttp/DavResource.kt index b00068f0..80f9343d 100644 --- a/src/main/kotlin/at/bitfire/dav4jvm/okhttp/DavResource.kt +++ b/src/main/kotlin/at/bitfire/dav4jvm/okhttp/DavResource.kt @@ -208,12 +208,7 @@ open class DavResource @JvmOverloads constructor( .build()) .execute() }.use { response -> - checkStatus(response) - if (response.code == HTTP_MULTISTATUS) - /* Multiple resources were to be affected by the MOVE, but errors on some - of them prevented the operation from taking place. - [_] (RFC 4918 9.9.4. Status Codes for MOVE Method) */ - throw HttpException(response) + checkStatus(response, multiStatusIsError = true) // update location location.resolve(response.header("Location") ?: destination.toString())?.let { @@ -250,13 +245,7 @@ open class DavResource @JvmOverloads constructor( .build()) .execute() }.use{ response -> - checkStatus(response) - - if (response.code == HTTP_MULTISTATUS) - /* Multiple resources were to be affected by the COPY, but errors on some - of them prevented the operation from taking place. - [_] (RFC 4918 9.8.5. Status Codes for COPY Method) */ - throw HttpException(response) + checkStatus(response, multiStatusIsError = true) callback.onResponse(response) } @@ -290,7 +279,7 @@ open class DavResource @JvmOverloads constructor( followRedirects { httpClient.newCall(request.build()).execute() }.use { response -> - checkStatus(response) + checkStatus(response, multiStatusIsError = true) callback.onResponse(response) } } @@ -541,13 +530,7 @@ open class DavResource @JvmOverloads constructor( httpClient.newCall(builder.build()).execute() }.use { response -> - checkStatus(response) - - if (response.code == HTTP_MULTISTATUS) - /* If an error occurs deleting a member resource (a resource other than - the resource identified in the Request-URI), then the response can be - a 207 (Multi-Status). […] (RFC 4918 9.6.1. DELETE for Collections) */ - throw HttpException(response) + checkStatus(response, multiStatusIsError = true) callback.onResponse(response) } @@ -662,13 +645,18 @@ open class DavResource @JvmOverloads constructor( /** * Checks the status from an HTTP response and throws an exception in case of an error. * + * @param multiStatusIsError when true, 207 Multi-Status is treated as an error * @throws HttpException in case of an HTTP error */ - protected fun checkStatus(response: Response) { - if (response.code / 100 == 2) - // everything OK + protected fun checkStatus(response: Response, multiStatusIsError: Boolean = false) { + // handle 2xx response codes + if (response.code / 100 == 2) { + if (response.code == HTTP_MULTISTATUS && multiStatusIsError) + throw HttpException(response) return + } + // handle other response codes throw when (response.code) { 401 -> UnauthorizedException(response) 403 -> ForbiddenException(response) diff --git a/src/test/kotlin/at/bitfire/dav4jvm/ktor/DavResourceTest.kt b/src/test/kotlin/at/bitfire/dav4jvm/ktor/DavResourceTest.kt index 533f557a..64d5532f 100644 --- a/src/test/kotlin/at/bitfire/dav4jvm/ktor/DavResourceTest.kt +++ b/src/test/kotlin/at/bitfire/dav4jvm/ktor/DavResourceTest.kt @@ -17,30 +17,12 @@ import at.bitfire.dav4jvm.property.webdav.DisplayName import at.bitfire.dav4jvm.property.webdav.GetETag import at.bitfire.dav4jvm.property.webdav.ResourceType import at.bitfire.dav4jvm.property.webdav.WebDAV -import io.ktor.client.HttpClient -import io.ktor.client.engine.mock.MockEngine -import io.ktor.client.engine.mock.respond -import io.ktor.client.engine.mock.respondError -import io.ktor.client.engine.mock.respondOk -import io.ktor.client.engine.mock.respondRedirect -import io.ktor.client.request.get -import io.ktor.client.request.prepareGet -import io.ktor.client.statement.bodyAsChannel -import io.ktor.client.statement.bodyAsText -import io.ktor.client.statement.request -import io.ktor.http.ContentType -import io.ktor.http.HeadersBuilder -import io.ktor.http.HttpHeaders -import io.ktor.http.HttpMethod -import io.ktor.http.HttpStatusCode -import io.ktor.http.URLBuilder -import io.ktor.http.Url -import io.ktor.http.content.TextContent -import io.ktor.http.contentType -import io.ktor.http.fullPath -import io.ktor.http.headersOf -import io.ktor.http.takeFrom -import io.ktor.http.withCharset +import io.ktor.client.* +import io.ktor.client.engine.mock.* +import io.ktor.client.request.* +import io.ktor.client.statement.* +import io.ktor.http.* +import io.ktor.http.content.* import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -532,6 +514,20 @@ class DavResourceTest { } } + @Test + fun `MkCol NEGATIVE TEST CASES 207 multi-status (MKCOL and MKCALENDAR should not return 207)`() = runTest { + val mockEngine = MockEngine { respond("", HttpStatusCode.MultiStatus) } + val httpClient = HttpClient(mockEngine) + var called = false + + try { + DavResource(httpClient, sampleUrl).mkCol("") { called = true } + fail("Expected HttpException") + } catch (_: HttpException) { + assertFalse(called) + } + } + @Test fun `Options with capabilities`() = runTest { val mockEngine = MockEngine {