sync: match a multiget reply by the resource, not by its spelling
The request href is ours, written into the REPORT body verbatim; the response href is the server's own spelling of it. Compared as raw encodedPath, a server answering /my@dav.ics for a requested /my%40dav.ics -- the case UrlUtils.equals exists for -- puts one resource in unsolicited *and* in missing, drops its intact body, and after three such runs quarantines it out of the download for good, because nothing on that side can refund the count. Our own names never carry an @, so this is about resources other clients created, which is most of an existing collection. The key is now scheme, host, port and the *decoded* path segments. A list rather than a joined string, so /a%2Fb does not collide with /a/b; with the host, so a same-path href from elsewhere cannot supply a body for our row. The trailing slash still distinguishes, as UrlUtils.equals also refuses to normalise it. The syncer stops spending an unrefundable count on a guess: when the server both omitted hrefs we asked for and volunteered hrefs we did not, "omitted" and "we did not recognise its spelling" are indistinguishable, so those are skipped rather than failed. A wasteful re-request is recoverable; a permanently dropped task is not.
This commit is contained in:
@@ -178,6 +178,26 @@ class CalendarCollection(
|
||||
*/
|
||||
override fun fetch(hrefs: List<HttpUrl>): Result<FetchResult> = fetch(hrefs, MULTIGET_BATCH)
|
||||
|
||||
/**
|
||||
* What makes two hrefs the same resource.
|
||||
*
|
||||
* ⚠️ Not the raw `encodedPath`. The request href is *ours*, written into the
|
||||
* REPORT body verbatim, and the response href is the server's own spelling
|
||||
* of it — a server that answers `/my@dav.ics` for a requested `/my%40dav.ics`
|
||||
* is the reason `UrlUtils.equals` exists at all. Compared raw, that resource
|
||||
* lands in `unsolicited` *and* in `missing`, its intact body is dropped, and
|
||||
* three such runs quarantine it out of the download for good, since nothing
|
||||
* on this side can refund the count.
|
||||
*
|
||||
* `pathSegments` is decoded, so both spellings agree. Keeping it a list, not
|
||||
* a joined string, stops `/a%2Fb` colliding with `/a/b`. Scheme, host and
|
||||
* port ride along because a path-only match would accept a body from another
|
||||
* host entirely. The trailing slash still distinguishes, exactly as
|
||||
* `UrlUtils.equals` refuses to normalise it.
|
||||
*/
|
||||
private fun identityOf(url: HttpUrl): List<String> =
|
||||
listOf(url.scheme, url.host, url.port.toString()) + url.pathSegments
|
||||
|
||||
/** [batchSize] is a seam for tests; production always uses [MULTIGET_BATCH]. */
|
||||
internal fun fetch(hrefs: List<HttpUrl>, batchSize: Int): Result<FetchResult> =
|
||||
runCatching {
|
||||
@@ -187,10 +207,10 @@ class CalendarCollection(
|
||||
val seen = mutableSetOf<HttpUrl>()
|
||||
|
||||
hrefs.chunked(batchSize).forEach { batch ->
|
||||
val wanted = batch.associateBy { it.encodedPath }
|
||||
val wanted = batch.associateBy { identityOf(it) }
|
||||
dav.multiget(batch, MIME_ICALENDAR, ICALENDAR_VERSION) { response, relation ->
|
||||
if (relation == Response.HrefRelation.SELF) return@multiget
|
||||
val asked = wanted[response.href.encodedPath]
|
||||
val asked = wanted[identityOf(response.href)]
|
||||
if (asked == null) {
|
||||
unsolicited += response.href
|
||||
return@multiget
|
||||
|
||||
@@ -101,6 +101,106 @@ END:VCALENDAR</C:calendar-data>
|
||||
assertThat(result.missing.map { it.encodedPath }).containsExactly("/dav/tasks/two.ics")
|
||||
}
|
||||
|
||||
@Test fun `a server that respells the path is still matched`() {
|
||||
server.enqueue(
|
||||
multistatus(
|
||||
"""
|
||||
<response>
|
||||
<href>/dav/tasks/my%40dav.ics</href>
|
||||
<propstat><prop>
|
||||
<getetag>"e1"</getetag>
|
||||
<C:calendar-data xmlns:C="urn:ietf:params:xml:ns:caldav">BEGIN:VCALENDAR
|
||||
END:VCALENDAR</C:calendar-data>
|
||||
</prop><status>HTTP/1.1 200 OK</status></propstat>
|
||||
</response>
|
||||
""",
|
||||
),
|
||||
)
|
||||
|
||||
val asked = href("my@dav.ics")
|
||||
val result = collection.fetch(listOf(asked)).getOrThrow()
|
||||
|
||||
// ⚠️ Compared raw, one resource lands in unsolicited *and* missing, its
|
||||
// body is dropped, and three such runs quarantine it out of the download
|
||||
// for good — nothing on that side can refund the count.
|
||||
assertThat(result.resources).hasSize(1)
|
||||
assertThat(result.missing).isEmpty()
|
||||
assertThat(result.unsolicited).isEmpty()
|
||||
assertThat(result.failed).isEmpty()
|
||||
// The href we asked for, not the server's spelling: apply is keyed on
|
||||
// the local row's href.
|
||||
assertThat(result.resources.single().href).isEqualTo(asked)
|
||||
}
|
||||
|
||||
@Test fun `the same path on another host is not our resource`() {
|
||||
server.enqueue(
|
||||
multistatus(
|
||||
"""
|
||||
<response>
|
||||
<href>https://evil.example.com/dav/tasks/one.ics</href>
|
||||
<propstat><prop>
|
||||
<getetag>"e1"</getetag>
|
||||
<C:calendar-data xmlns:C="urn:ietf:params:xml:ns:caldav">BEGIN:VCALENDAR
|
||||
END:VCALENDAR</C:calendar-data>
|
||||
</prop><status>HTTP/1.1 200 OK</status></propstat>
|
||||
</response>
|
||||
""",
|
||||
),
|
||||
)
|
||||
|
||||
val result = collection.fetch(listOf(href("one.ics"))).getOrThrow()
|
||||
|
||||
// Matching on the path alone would apply one host's body to another
|
||||
// host's row.
|
||||
assertThat(result.resources).isEmpty()
|
||||
assertThat(result.unsolicited.map { it.host }).containsExactly("evil.example.com")
|
||||
assertThat(result.missing).hasSize(1)
|
||||
}
|
||||
|
||||
@Test fun `a trailing slash is still a different resource`() {
|
||||
server.enqueue(
|
||||
multistatus(
|
||||
"""
|
||||
<response>
|
||||
<href>/dav/tasks/one.ics/</href>
|
||||
<propstat><prop><getetag>"e1"</getetag></prop><status>HTTP/1.1 200 OK</status></propstat>
|
||||
</response>
|
||||
""",
|
||||
),
|
||||
)
|
||||
|
||||
val result = collection.fetch(listOf(href("one.ics"))).getOrThrow()
|
||||
|
||||
// Deliberately not normalised, as UrlUtils.equals also refuses to.
|
||||
assertThat(result.resources).isEmpty()
|
||||
assertThat(result.missing).hasSize(1)
|
||||
}
|
||||
|
||||
@Test fun `an encoded slash is not the same as a real one`() {
|
||||
server.enqueue(
|
||||
multistatus(
|
||||
"""
|
||||
<response>
|
||||
<href>/dav/tasks/a/b.ics</href>
|
||||
<propstat><prop>
|
||||
<getetag>"e1"</getetag>
|
||||
<C:calendar-data xmlns:C="urn:ietf:params:xml:ns:caldav">BEGIN:VCALENDAR
|
||||
END:VCALENDAR</C:calendar-data>
|
||||
</prop><status>HTTP/1.1 200 OK</status></propstat>
|
||||
</response>
|
||||
""",
|
||||
),
|
||||
)
|
||||
|
||||
val asked = server.url("/dav/tasks/a%2Fb.ics")
|
||||
val result = collection.fetch(listOf(asked)).getOrThrow()
|
||||
|
||||
// One segment "a/b" is not two segments "a" and "b" — which is why the
|
||||
// key stays a list rather than a joined string.
|
||||
assertThat(result.resources).isEmpty()
|
||||
assertThat(result.missing).containsExactly(asked)
|
||||
}
|
||||
|
||||
@Test fun `a refused resource is reported as failed, not as missing`() {
|
||||
server.enqueue(
|
||||
multistatus(
|
||||
|
||||
Reference in New Issue
Block a user