First request to a CredentialsAuthenticator or AsyncCredentialsAuthenticator fails if the body is too large

[owainhunt]

Describe the bug

When using a route protected by an AsyncCredentialsAuthenticator, the first request to the route fails if the body is too large (in my testing, above 1795 bytes). Subsequent requests to the same route succeed as expected.

It appears to fail at the point the authenticator attempts to decode the request body - see Authentication+Concurrency.swift:AsyncCredentialsAuthenticator.authenticate(request:):

vapor/Sources/Vapor/Concurrency/Authentication+Concurrency.swift

Line 87 in 8075031

credentials = try request.content.decode(Credentials.self)

The error message suggests the request body is empty. Indeed if you put a breakpoint at that line, po request.body.string returns nil.

Error message is as follows:

"message" : "Request body is empty. If you're trying to stream the body, decoding streaming bodies not supported",
  "request-id" : "38650034-1563-4198-9482-54C36E88177F",
  "severity" : "DEBUG",
  "sourceLocation" : {
    "file" : "Vapor/Request/Request.swift",
    "function" : "decode(_:using:)",
    "line" : 77
  },

To Reproduce

Define an AsyncCredentialsAuthenticator:

class DummyAuth: AsyncCredentialsAuthenticator {
    typealias Credentials = String
    func authenticate(credentials: Credentials, for request: Request) async throws {
        request.auth.login(credentials)
    }
}

Protect a route with an instance of that authenticator:

app.grouped(Dummy()).grouped(String.guardMiddleware()).post("blah") { req -> Response in
        Response(status: .ok)
    }

Run the app and make two requests, with a body length of greater than 1795 bytes.

Expected behavior

Both requests will succeed with a 200 response.

Actual behaviour

First request fails with a 401:

{
    "error": true,
    "reason": "String not authenticated."
}

Second request succeeds with a 200.

Environment

• Vapor Framework version: 4.52.2
• OS version: Monterey 12.0.1

[0xTim]

@owainhunt Just to confirm the non-async version works fine?

[owainhunt]

@0xTim I'd not looked at the non-async version, but in a quick test the below code behaves in exactly the same way - with a body above a certain size, the first request fails but subsequent requests succeed.

class DummyAuthFuture: CredentialsAuthenticator {
    
    typealias Credentials = String
    
    func authenticate(credentials: Credentials, for request: Request) -> EventLoopFuture<Void> {
        request.auth.login(credentials)
        return request.eventLoop.makeSucceededFuture(())
    }
}

[0xTim]

I wonder if this is related to #2494 and that over a certain size (probably the size of the ByteBuffer) it fails to decode the entire body. Does adding request.body.collect(max: nil) to the middleware fix it?

[owainhunt]

These seem to work in some basic testing:

extension AsyncCredentialsAuthenticator {
    public func authenticate(request: Request) async throws {
        
        _ = try await request.body.collect(max: nil).get()

        let credentials: Credentials
        do {
            credentials = try request.content.decode(Credentials.self)
        } catch {
            return
        }
        return try await self.authenticate(credentials: credentials, for: request)
    }
}

extension CredentialsAuthenticator {
    public func authenticate(request: Request) -> EventLoopFuture<Void> {
        
        return request.body.collect(max: nil).flatMap { _ -> EventLoopFuture<Void> in
        
            let credentials: Credentials
            do {
                credentials = try request.content.decode(Credentials.self)
            } catch {
                return request.eventLoop.makeSucceededFuture(())
            }
            return self.authenticate(credentials: credentials, for: request)
        }
    }
}

Still not sure why it would previously fail the first time and then work on subsequent requests, though.

[0xTim]

My guess would be the event loop buffer would be increased after the decode or something like that. It would have to be something specific to the event loop and not the request.

Whatever it is it's definitely a bug

[grennis]

I have also seen this problem, with somewhat different symptoms because I am not using an authenticated route. The first request with a large post body (in my case, ~600KB) will often hang (like 1 in 10 times) because my route handler is never called. So the incoming request is logged and then it just seems to get dropped. Note that my middleware handlers are called as normal. This happens on both macOS and Linux (Heroku).

It's not always the first request - it does happen on subsequent requests, but very much more rarely.

After reading this issue, I tried using this as the first middleware as a workaround, and it did indeed fix the problem - no more dropped requests, on any os:

class ConsumeBodyMiddleware: Middleware {
    func respond(to request: Request, chainingTo next: Responder) -> EventLoopFuture<Response> {
        request.body.collect(max: ("50mb" as ByteCount).value)
            .flatMap { _ in
                next.respond(to: request)
            }
    }
}

[0xTim]

The underlying issue is #2742, will close it in favour of that one. It's being looked at

[marius-se]

@owainhunt @grennis can you still reproduce this (when skipping the collect workaround)? I'm currently investigating this issue, but couldn't reproduce it so far.