Skip to content

fix: remove double execute() in HTTP interceptor chain causing media uploads to fail#384

Open
cryptomentor-de wants to merge 1 commit into
ghostbyte-dev:mainfrom
cryptomentor-de:fix/double-execute-upload-auth
Open

fix: remove double execute() in HTTP interceptor chain causing media uploads to fail#384
cryptomentor-de wants to merge 1 commit into
ghostbyte-dev:mainfrom
cryptomentor-de:fix/double-execute-upload-auth

Conversation

@cryptomentor-de

Copy link
Copy Markdown

Summary

Media uploads (POST /api/v2/media) fail intermittently — especially when uploading multiple files at once. Single uploads sometimes succeed, mass uploads almost always fail.

Root Cause

The HttpSend interceptor in AppComponent calls execute(request) twice per request:

// BEFORE (broken)
plugin(HttpSend).intercept { request ->
    with(session) { intercept(request) }        // execute() #1 — NO Bearer token
    with(authInterceptor) { intercept(request) } // execute() #2 — with Bearer token
}

Session.intercept() sets the server URL host and then calls execute(request), which sends the actual network request without the Authorization header. The server receives an unauthenticated request and returns 401/500. Because expectSuccess = true is configured on the HttpClient, ktor throws a ServerResponseException on the 500 response — so AuthInterceptor.intercept() on the next line is never reached.

For multipart uploads in particular, even in cases where the exception doesn't propagate, the first execute() call consumes the streaming request body. The second execute() in AuthInterceptor then sends an empty body to the server, which rejects it.

I was able to confirm this by adding request-level logging on the server side. Every failed upload arrived with Authorization: (missing). The few that eventually succeeded arrived with Authorization: Bearer <token> — sent by AuthInterceptor in the rare cases where the first execute did not throw.

Fix

Inline the URL-host substitution from Session directly into the HttpSend interceptor block, without calling execute(). Then delegate to AuthInterceptor.intercept() which adds the Bearer token and performs a single execute():

// AFTER (fixed)
plugin(HttpSend).intercept { request ->
    session.credentials.value?.let { creds ->
        if (request.url.host != "api.fedisea.surf" && request.url.host != "pixelfed.org") {
            request.url.set(host = Url(creds.serverUrl).host)
        }
    }
    with(authInterceptor) { intercept(request) }
}

Session.intercept() (the member extension on Sender) is no longer needed and is removed along with its now-unused imports.

Test plan

  • Single photo upload succeeds
  • Single video upload succeeds
  • Mass upload (multiple photos + videos selected at once) succeeds without any 401/500 errors
  • Token refresh flow still works (expired token → refresh → retry)
  • Login / logout unaffected

Session.intercept() called execute(request) to set the server URL, which
sent every request to the server once without a Bearer token before
AuthInterceptor could add it. This caused the server to receive two
requests per call: one unauthenticated (returning 401/500) and one
authenticated.

For multipart media uploads the double-execute is especially harmful:
the first unauthenticated execute either triggers an exception (due to
expectSuccess = true) that prevents AuthInterceptor from running at all,
or it consumes the request body so the second execute sends an empty
payload. Either way the upload fails.

Fix: inline the URL-host substitution from Session directly into the
HttpSend interceptor block in AppComponent, then call only
AuthInterceptor.intercept() which adds the Bearer token and performs a
single execute(). The Session.intercept() member extension function is
no longer needed and is removed along with its now-unused imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant