Skip to content

Split the media delegate into MediaProcessor and MediaUploader - #621

Draft
jkmassel wants to merge 21 commits into
fix/register-core-media-upload-middlewarefrom
refactor/media-processor-uploader
Draft

Split the media delegate into MediaProcessor and MediaUploader#621
jkmassel wants to merge 21 commits into
fix/register-core-media-upload-middlewarefrom
refactor/media-processor-uploader

Conversation

@jkmassel

@jkmassel jkmassel commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Stacked on #594. Makes performing a media upload — and retrying it — a single, all-or-nothing responsibility: either GutenbergKit performs the upload and owns its retries, or the host does (say, to run it through its own networking so it can log every request). Both go to the same configured site; the only difference is who executes the requests. There's no in-between where the host performs the upload but GutenbergKit retries it.

Summary

Two protocols replace the single MediaUploadDelegate:

  • MediaProcessor — transform the file before upload; GutenbergKit performs the upload and owns its retries. The common, safe extension point.
  • MediaUploader — perform the upload yourself (your own networking, logging, retry policy, a background session) and own its whole lifecycle: retries, recovery, and cleanup. Receives a MediaUpload value carrying the file, its metadata, the editor's non-file form fields (post, additionalData), and the request query (?_embed).

EditorViewController / GutenbergView own whichever handler you set — their properties are strong on both platforms, so you can assign one and drop your own reference (just don't strongly retain the editor back).

Breaking: hosts migrate mediaUploadDelegatemediaProcessor / mediaUploader.

The problem with the old design

When an upload fatals in server-side post-processing, it has to be retried: core retries POST …/post-process up to 5×, and cleans up the orphan if that fails.

MediaUploadDelegate.uploadFile let a host perform the upload itself by returning the raw response it received. But that split one upload's HTTP across two owners: the host performed the POST /wp/v2/media, then core — reading that raw response — drove the post-process retries (and the orphan cleanup) behind it. A host that took over uploads to run them through its own stack still didn't own the retries; those went out through the browser, not the host. Delivery and its retries were owned by different parties.

The fix

Make the upload and its retries one unit with one owner:

  • MediaProcessor (handlesFile, processFile) only transforms the file. It never performs the upload, so GutenbergKit performs it and owns the retries. The extension point almost every host wants.
  • MediaUploader (upload(_:)) performs the upload on the host's own stack and owns the whole lifecycle. upload returns the finished attachment or throws — there's no raw response for core to retry behind it, so the host drives its own post-process recovery and force-deletes its own orphan on terminal failure. The MediaUpload it receives carries everything needed to reproduce a native request — the file, the editor's post / additionalData fields, and the ?_embed query — so a host upload attaches to its post instead of landing as an unattached orphan.

So it's all-or-nothing: the host performs the upload and its retries, or GutenbergKit does — never a split. An uploader and GutenbergKit's built-in default both target the same configured site; the choice is only who executes the requests.

Media deletes always relay to the default uploader (the configured site) — every attachment lives there, even one a host uploader delivered, so there's no per-host delete path. The server starts if either handler is set and builds a default uploader whenever site credentials are present (it delivers GutenbergKit's own uploads and relays every delete). Because deletes need it, a mediaUploader set without site credentials is a configuration error and traps at startup rather than starting a server whose every delete would 500. MediaUploadResponse is now internal — it's no longer on any public API.

Accepted Risk / Out of Scope

  • The configured-site delete relay isn't scoped. GutenbergKit relays core's cleanup DELETE to the configured site, but the relay can't distinguish it from any other DELETE the WebView sends — a client-side-compromised editor (a supply-chain-tampered JS bundle, or editor XSS) holding the loopback token could force-delete arbitrary media there. We accept this: such a script already has broad write access via allowed methods, and a server-side compromise (a malicious plugin) deletes media directly without the editor. An earlier revision carried a per-session ledger to scope the relay; it's dropped as not worth the cost for a client-side-only threat.

Test Plan

  • iOS host suite green
  • Android unit suites green — MediaUploadServer, GutenbergView
  • SwiftLint + Detekt clean
  • iOS demo app builds (xcodebuild, Xcode 26.4.1); Android demo builds (Detekt compiles it)
  • Migrate mediaUploadDelegate in WordPress-iOS / WordPress-Android / Jetpack

Related

dcalhoun and others added 9 commits August 21, 2026 09:12
`handleDelete` went straight to the default uploader, unlike `handleUpload`
which offers the work to the delegate first. A host whose `uploadFile`
uploads to its own media service holds an ID only it can resolve, so
deleting through the default uploader would address the wrong site.

Add `deleteFile(attachmentId:)` to `MediaUploadDelegate` on both platforms,
defaulted to nil so existing hosts are unaffected, and try it before falling
back. Rename the handler to `handleMediaDelete`, since it deletes an
attachment rather than an upload and no longer mirrors `handleUpload`'s
signature.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF
Drop the two notes explaining how the simulator produces its 500 and why a
fatal response reads as a CORS error — implementation detail that belongs in
the plugin, not the guide. Drop the orphaned-server and stale-credential
troubleshooting entries; those are environment problems to address on their
own. Also drop a comment restating what the adjacent condition already says,
and reword the make target's help text.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF
`nativeMediaUploadMiddleware` mixed dispatch with the whole upload
implementation, so adding the deletion path left the two handled
asymmetrically — one extracted, one inline.

Extract `nativeMediaUpload` alongside `nativeMediaDelete`, both returning
null when a request is not theirs, leaving the middleware as a short
dispatcher. No behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF
`handleMediaDelete` caught only `IOException`, so a delegate's `deleteFile`
throwing anything else — `IllegalStateException`, a JSON error — escaped to
`HttpServer.resolveResponse` and returned a plain-text 500. The editor's
`nativeMediaDelete` then failed on `response.json()` and reported
`invalid_json` rather than the delegate's actual failure.

Catch `Exception` and rethrow `CancellationException`, matching
`passthroughResponse` and iOS's untyped `catch`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr
`relayResponse` prepended `Content-Type: application/json` to an array of the
response's own headers, and `HTTPResponse` serializes every entry it is given.
A delegate returning its own `Content-Type` therefore put the header on the
wire twice, which URLSession surfaces as "application/json, text/plain".
Android's map merge already overrode instead, so the two platforms disagreed
on the same public API.

Skip the default when the response already carries the name, matching Android.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr
`deleteFile` is called for every deletion, including attachments the delegate
declined at upload time — an attachment ID carries no MIME type or filename,
so there is no `handlesFile` gate to apply. A delegate answering for one of
those leaves the real WordPress attachment undeleted, which is the orphan the
cleanup exists to remove.

Returning nil already falls through to the default uploader; document that as
the signal for an unrecognized ID.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr
The credentials path was interpolated into the `node -e` source as a
single-quoted JS string literal, so a checkout under a path containing a quote
or backslash produced a SyntaxError stack trace instead of the intended
"could not read authHeader" message.

Pass it through `process.argv` and single-quote the script body so the shell
does not expand it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr
`relayResponse` merged the JSON default with the response's own headers via
Kotlin's map merge, which only overrides on an exact key match. A delegate
returning `content-type` therefore produced a two-entry map, and
`serializeResponse` writes every entry, putting the header on the wire twice —
the WebView sees "application/json, text/plain".

This is the same defect `0bf40ad3` fixed on iOS, which the map merge was
believed to already handle. Skip the default when the response carries the name
under any casing, matching iOS and the case-insensitive lookups `HttpServer`
already uses.

Add the Android counterpart to the iOS `delegateContentTypeWins` test. It
asserts on the raw header lines rather than the parsed map, which lowercases
keys into a map and would collapse the duplicate — hiding the very bug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVdVBubbCDp7mRXSWkcHHm
The native upload middleware runs below core's mediaUploadMiddleware, which
forwards every upload as `parse: false` and reads `x-wp-upload-attachment-id`
off a rejected Response to retry post-process. Logging the initial 5xx at
error level reported a failure before recovery ran, so every upload that
silently recovered still emitted an error. Reject without logging, matching
the nativeMediaDelete sibling — the initial 5xx is a handoff to core's retry,
not an outcome.
@jkmassel jkmassel added [Type] Breaking Change For PRs that introduce a change that will break existing functionality iOS Android labels Sep 3, 2026
@jkmassel jkmassel self-assigned this Sep 3, 2026
@wpmobilebot

wpmobilebot commented Sep 3, 2026

Copy link
Copy Markdown

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/621")

Built from 2bbc65d

@jkmassel
jkmassel force-pushed the refactor/media-processor-uploader branch 4 times, most recently from 9fe481e to 9f00177 Compare September 3, 2026 21:50
Make performing a media upload -- and retrying it -- a single, all-or-nothing
responsibility: either GutenbergKit performs the upload and owns its retries, or
the host does (say, to run it through its own networking so it can log every
request). Both go to the same configured site; the only difference is who executes
the requests. There is no in-between where the host performs the upload but
GutenbergKit retries it.

When an upload fatals in server-side post-processing it has to be retried: core
retries POST .../post-process up to 5x, and cleans up the orphan if that fails.
The old `MediaUploadDelegate.uploadFile` let a host perform the upload itself by
returning the raw response it received -- which split one upload's HTTP across two
owners: the host performed the POST /wp/v2/media, then core, reading that raw
response, drove the post-process retries (and the orphan cleanup) behind it. A
host that took over uploads to run them through its own stack still didn't own the
retries; those went out through the browser, not the host. Delivery and its
retries were owned by different parties.

Make the upload and its retries one unit with one owner:

- `MediaProcessor` (handlesFile, processFile) only transforms the file. It never
  performs the upload, so GutenbergKit performs it and owns the retries -- the
  extension point almost every host wants.
- `MediaUploader` (upload) performs the upload on the host's own stack (its
  networking, logging, retry policy, a background session) and owns the whole
  lifecycle. `upload` returns the finished attachment or throws: there's no raw
  response for core to retry behind it, so the host drives its own post-process
  recovery and force-deletes its own orphan on terminal failure.

All-or-nothing: the host performs the upload and its retries, or GutenbergKit does
-- never a split. An uploader and GutenbergKit's built-in default both target the
same configured site; the choice is only who executes the requests.

Media deletes always relay to the default uploader (the configured site): every
attachment lives there, even one a host uploader delivered, so there is no per-host
delete path. The relay is left unscoped -- core issues its cleanup DELETE there,
but the relay can't tell it from any other DELETE the WebView sends, so a
client-side-compromised editor holding the loopback token could force-delete media
on the site. Accepted -- such a script already has broad write access, and a
server-side compromise deletes media directly without the editor. An earlier
revision carried a per-session ledger to scope the relay; dropped as not worth the
cost for a client-side-only threat.

`EditorViewController`/`GutenbergView` expose `mediaProcessor` + `mediaUploader`
in place of `mediaUploadDelegate`; the server starts if either is set and builds a
default uploader whenever site credentials are present (it delivers GutenbergKit's
own uploads and relays every media delete). `MediaUploadResponse` drops to internal
-- it is no longer on any public API. Both demos and all tests move to the new
protocols.

Breaking change: hosts must migrate `mediaUploadDelegate` (WordPress-iOS/Android,
Jetpack). iOS and Android suites green; SwiftLint and Detekt clean.
@jkmassel
jkmassel force-pushed the refactor/media-processor-uploader branch 5 times, most recently from 0951f5a to d0ee224 Compare September 4, 2026 18:26
Follow-ups from review of the delegate split:

- Ownership: `EditorViewController` holds `mediaProcessor`/`mediaUploader`
  strongly now, not `weak`, matching Android — a host can assign one and drop its
  own reference without native media silently stopping mid-session. The load-time
  `wasAssigned` traps that only guarded the old release-before-load footgun are
  gone; a new `EditorMediaHandlerOwnershipTests` pins the retention.

- Credentials: setting a `mediaUploader` without an auth header now traps at
  startup. Media deletes always relay to the configured site, so an uploader
  without credentials could upload but every delete would 500 — a configuration
  error, surfaced like the late-assignment trap rather than a half-working server.

- Upload context: `MediaUploader.upload` takes a `MediaUpload` value type carrying
  the file plus the editor's non-file form fields (`post`, additionalData) and the
  request query (`?_embed`). The old signature dropped them, so a host upload
  created an unattached orphan and lost the query — the default path already
  forwarded them.

- `handlesFile` is consulted once per upload, not twice: the admission gate's
  decision is threaded into the pipeline instead of recomputed, so the two steps
  can't disagree.

- Rename the internal configured-site client `DefaultMediaUploader` →
  `InternalMediaClient` (field `defaultUploader` → `internalClient`). It's
  GutenbergKit's own client for the configured site — it delivers GutenbergKit-owned
  uploads, relays every delete, and does passthrough — not a "default" that a host
  `mediaUploader` overrides; the host takes a different path entirely. Internal-only;
  no public-API change.

- Cleanup: drop the now-dead `Content-Type` dedup in `relayResponse` (relayed
  bodies are always WordPress REST JSON and relayed headers are a
  content-type-free allowlist, so the JSON default always applies); drop the stale
  `delegate` vocabulary (error string, doc comment); rename
  `MediaUploadDelegate.swift` to `MediaHandlers.swift`. `MediaUploadServer` now
  requires a non-null `cacheDir` and `internalClient`: the server never starts
  without a cache dir or site credentials, so its staging directory is
  correct-by-construction (no `java.io.tmpdir` fallback), and its delete / passthrough
  / upload paths drop the dead "no uploader configured" guards.

- Tests: the host-`mediaUploader` suite now asserts the uploader receives the actual
  file bytes (not just metadata), that a processor's processed file and new metadata
  reach the uploader, and that a throwing uploader surfaces as a relayed 500.

iOS and Android suites green; SwiftLint and Detekt clean.
MediaUpload.fields was a last-wins map, so a repeated field name (e.g. a
`field[]` array) collapsed to its final value before a host MediaUploader
saw it — diverging from GutenbergKit's own upload path, which preserves
repeats. Pass an ordered list of (name, value) pairs on both platforms.
UploadContext held the processor and uploader `weak`, and each was read
twice per request — once at the admission gate, once at delivery. Those
reads are separated by a synchronous disk copy and an unbounded
`processFile`, so a host that released its handler in that window (the
user closing the editor mid-transcode) changed the answer between them:
a file admitted for processing was forwarded unprocessed, and an upload
gated on a host uploader was delivered by GutenbergKit itself — creating
an attachment on the configured site that the host never learns about,
behind an uploader documented as keeping GutenbergKit "out of the
network entirely".

Hold all three strongly, as Android already does. The `weak` was
load-bearing when `EditorViewController.mediaUploadDelegate` was itself
`weak` and this was the only strong path; 4a03bcc made those properties
strong and left it behind. It no longer prevents a cycle — a host object
retaining the view controller already forms
`EditorViewController -> mediaUploader -> EditorViewController` through
the view controller's own strong property, which this container can
neither create nor prevent.

Immutable strong references also make the two reads agree by
construction, so `handlesFile` admission and delivery can't disagree.
UploadContext becomes a struct and drops its `@unchecked Sendable`
opt-out: both protocols are `Sendable` and InternalMediaClient is
`@unchecked Sendable`, so it is implicitly Sendable.

`doesNotStronglyRetainProcessor` pinned the vestigial invariant, so it is
replaced by `retainsProcessorForServerLifetime`, asserting both halves —
the server owns its processor while it runs, and releases it when the
server goes away. `uploaderReleasedMidRequestStillDelivers` covers the
bug directly; against the previous commit it fails with the real
symptom, the host uploader bypassed and passthroughUpload called.
Both delivery paths could put bytes on the wire after the editor was
gone. `EditorViewController.deinit` calls `stop()`, which cancels the
in-flight connection tasks, but Swift cancellation is cooperative:
`writeStream` is an uninterruptible read loop and a host's `processFile`
need not check at all, so a handler can reach delivery well after
teardown. Whether the request then actually reached WordPress rested
entirely on URLSession noticing the cancellation.

That is not a guarantee the server can rely on. `URLSessionProtocol` is
public and documented for dependency injection, and the obvious
conformance for a host wrapping a callback-based stack —
`withCheckedThrowingContinuation` around a completion handler — has no
cancellation awareness at all. Such a host would upload deterministically
after teardown, and the response is discarded either way, leaving an
attachment on the site that nothing cleans up.

Check cancellation explicitly before delivery in `processAndUpload` and
before `passthroughUpload`, so the guarantee comes from this file rather
than from the HTTP client's behaviour. CancellationError is already
handled quietly by `uploadErrorResponse`, and HTTPServer drops the
response for a cancelled task.
`passthroughResponse` and `handleMediaDelete` took the whole
UploadContext and touched only `internalClient`. Pass that directly.

On the delete path this is more than tidiness. Every attachment lives on
the configured site — even one a host uploader delivered — so a deletion
always relays through the internal client, never the uploader. That was
a convention the signature let you break; now it is a fact the compiler
enforces.

`handleUpload` and `processAndUpload` keep the context: they genuinely
need all three, and spelling them out would push processAndUpload to
nine parameters.
The closure form of `start` can't capture the object that owns the
server: the closure has to exist before the server does, and retrofitting
`self` would form `owner -> HTTPServer -> handler -> owner`, so the
owner's deinit — and its `stop()` — would never run. A consumer with
dependencies to hold therefore ends up with static functions threading a
context parameter through every call, which is how MediaUploadServer is
written today.

Add an `HTTPRequestHandler` protocol and a `start` overload that takes
one. The dependencies become stored properties and the request logic
becomes instance methods. The protocol is deliberately not
`AnyObject`-constrained: a struct conformer cannot participate in a
reference cycle at all, so the ownership question doesn't arise. A final
class works too, under the same leaf discipline HTTPServerDelegate
already documents.

The closure overload is unchanged and forwards to the same code path, so
this is purely additive — no existing caller, test, or the debug server
is affected. Request handling is mandatory, so it can't be a defaulted
HTTPServerDelegate method the way optional customization points are;
hence an overload rather than a new delegate requirement.
Every request function was `private static` taking an UploadContext, for
one reason: the handler closure has to exist before MediaUploadServer
does, so it couldn't capture `self`, and capturing it later would form
`MediaUploadServer -> HTTPServer -> handler -> MediaUploadServer` and
stop `deinit` from ever running `stop()`.

Move them onto a `Handler` struct conforming to the HTTPRequestHandler
protocol added in the previous commit. The dependencies become stored
properties, so the five request functions become instance methods and
drop their context parameter; UploadContext is deleted, since the handler
now *is* the context. A struct can't participate in a reference cycle, so
the constraint that forced the statics is gone rather than worked around.

The statics that remain — errorResponse, relayResponse, attachmentId,
formFields, sanitizeFilename, writeStream, cleanOrphanedUploads — are
pure functions of their arguments. `static` there is not a workaround; it
is the honest signal that they depend on nothing, which is now a
meaningful distinction rather than an artifact of the closure.

No behaviour change and no test changes: the only entry point is
`MediaUploadServer.start`, whose signature is untouched. Reviewing with
`--color-moved` will help — most of the diff is the request block moving
into the struct and gaining a level of indentation.
MediaUpload.fields was `[(name: String, value: String)]` on iOS. Tuples
are not nominal types, so a tuple-typed stored property permanently
blocks synthesized Equatable, Hashable and Codable on MediaUpload —
inside GutenbergKit as well as for hosts, and retroactively, so no later
conformance can recover it. That matters for the offline queue the
MediaUploader docs advertise as a motivating use case: a host that wants
to persist a pending upload's fields has to hand-roll a mirror type.

Unlike the missing public memberwise init on MediaUpload — which stays
internal, matching how this library treats outbound types, and which
could be added later without breaking anyone — this one is not fixable
additively. Changing `fields` after release is a source break for every
host, so it happens now or not at all.

Introduce MediaUploadField (Sendable, Hashable, Codable, public init) and
use it on both platforms. Android had no equivalent defect — Kotlin's
Pair is nominal — but the same change lands there for parity, and
`field.name`/`field.value` reads better than `first`/`second` in a host's
upload code. Kotlin data class destructuring means the multipart writers
are unchanged.

MediaUpload itself is deliberately left non-Codable: it carries a
`fileURL` pointing at a GutenbergKit temp file that will not exist after
a relaunch, so a serialized MediaUpload would be a trap. A host queueing
an upload should copy the bytes and persist the fields, which this type
now supports.
Android refused a mediaUploader when either `siteApiRoot` or `authHeader`
was missing; iOS checked only `authHeader`. So a host with valid
credentials but no site root crashed on Android and started a server on
iOS — the same configuration, opposite outcomes, on a pair of fields that
are both required for the internal media client to reach the configured
site at all.

Gate iOS on both. The types differ — `siteApiRoot` is a `URL` on iOS and
a `String` on Android — so the equivalent of Android's `isEmpty()` is
"not absolute": a URL with no scheme or host cannot address the site, and
every media request built from it fails at the URLSession layer.

Also covers the arm nothing tested. `GutenbergViewUploadServerTest` only
exercised the missing-authHeader case; add its siteApiRoot sibling. There
is no iOS equivalent because `precondition` takes the test process down,
where Kotlin's `check` throws catchably.

The iOS comment claimed every delete "would 500" without credentials.
That is right for a missing site root, where okhttp/URLSession reject the
schemeless URL, but a missing auth header relays WordPress's 401 instead.
Say "would fail", which is true of both.
The previous commit claimed this policy was untestable on iOS because
`precondition` takes the test process down. That was wrong: Swift Testing
has exit tests, which run the body in a child process.

The real obstacle was narrower. Exit tests are unavailable on iOS and the
simulator ("Exit tests are not available on this platform"), and the
policy lived in EditorViewController, which is `#if canImport(UIKit)` and
therefore absent from the macOS host — so the one platform that can run
exit tests couldn't see the code. The intersection was empty because of
where the code sat, not because of the tool.

Move the decision into MediaServerCredentials, outside the UIKit gate,
and have startUploadServer call it. The predicate and the fail-fast are
now both reachable from the host suite: six tests pin the predicate
(including the two arms of the site-root check that a `URL` makes
different from Android's `String`), and two exit tests pin the trap
itself. Neutering the precondition fails both, so they are not vacuous.

This also gives the crash policy a named home. It diverged silently
between iOS and Android once already; a host-testable predicate is
harder to let drift again.
`formFields` decodes each non-file form value as UTF-8, which substitutes
U+FFFD on malformed input. That is lossless today, but only because of an
invariant nothing in the code states or enforces: the sole client is the
editor's browser FormData. The server binds to loopback behind a
per-session token; a FormData string value is a USVString, already
well-formed at append time; and its only way to carry arbitrary bytes is
a Blob, which always gets a filename and is filtered out of `extraParts`.

Write that down on both platforms, including the part that makes it
matter — if it stops holding, the two platforms are lossy *differently*
(for ED A0 80, Swift's maximal-subpart rule yields three replacement
characters where Java's decoder yields one), so there is no single
behaviour that could be documented instead.

Also reword the raw-bytes comment on the re-encode path. "So a non-UTF-8
value is forwarded verbatim" read as though malformed values were
expected, which made the two delivery paths look contradictory. The
actual hazard is the failable `String(data:encoding:)` returning nil and
an obvious `?? ""` dropping the whole value; the reason to keep bytes is
that the re-encode should stay byte-identical to the passthrough it
stands in for.

Cover the partition rather than the decode, since the partition is what
makes the invariant true: a request carrying a second, Blob-shaped part
whose bytes are not valid UTF-8 must not surface that part in `fields`.
Both tests fail when the filename filter is relaxed, so neither is
vacuous. Neither asserts what becomes of that second part — it is
currently dropped rather than relayed, which is a separate open question.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Android iOS [Type] Breaking Change For PRs that introduce a change that will break existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants