vapor / postgres-kit

🐘 Non-blocking, event-driven Swift client for PostgreSQL.
MIT License
186 stars 70 forks source link

Occasional Encoding Error #256

Closed RussBaz closed 7 months ago

RussBaz commented 9 months ago

Describe the bug

Occasionally, my Vapor server will write the following error in the log (manually formatted):

[ WARNING ] PSQLError(code: server, 
    serverInfo: [
        sqlState: 22021, 
        file: mbutils.c, 
        line: 1669, 
        message: invalid byte sequence for encoding "UTF8": 0x00, 
        routine: report_invalid_encoding, 
        localizedSeverity: ERROR, 
        severity: ERROR, 
        locationContext: unnamed portal parameter $2
    ], 
    triggeredFromRequestInFile: PostgresKit/PostgresDatabase+SQL.swift, 
    line: 53, 
    query: PostgresQuery(
        sql: INSERT INTO "otp_configs" ("id", "key", "created_at", "updated_at", "user_id") VALUES ($1, $2, $3, $4, $5) RETURNING "id", 
        binds: [
            (****; UUID; format: binary), 
            (****; TEXT; format: binary), 
            (****; TIMESTAMPTZ; format: binary), 
            (****; TIMESTAMPTZ; format: binary), 
            (****; UUID; format: binary)
        ]
    )
)

and I do not know how to reproduce it or what causes it.

This query is executed from an inside a transaction from a helper function:

func createOtpUri(_ req: Request, for user: User, existingRecord: OTP.Record? = nil) async throws -> String? {
    try await req.db.transaction { db in
        if let existingRecord {
            try await existingRecord.delete(on: db)
        } else if let record = try await OTP.Record.query(on: db)
            .filter(\.$user.$id == user.id)
            .with(\.$user)
            .first()
        {
            guard record.verifiedAt == nil else {
                return nil
            }

            try await record.delete(on: db)
        }

        let otp = try OTP(email: user.email)
        let record = try OTP.Record(otp, for: user)

        try await record.create(on: db)

        return try otp.uri
    }
}

To Reproduce

I need help debugging this issue, as I do not know how to reproduce the error.

Also, I have not tried this yet with a more modern version of Postgres or on Apple Silicon.

In case you think this is related to the input data, here is the cleaned up definition of the OTP struct and the model that is being inserted:

struct OTP {
    private let key: String
    private let email: String

    init(email: String, key: String? = nil) throws {
        self.email = email

        if let key {
            guard let _ = SymmetricKey(key) else {
                throw Abort(.internalServerError)
            }
            self.key = key
        } else {
            self.key = SymmetricKey(size: .bits192).description
        }
    }
}

extension OTP {
    final class Record: Model {
        static let schema = "otp_configs"

        @ID(key: .id)
        var id: UUID?

        @Field(key: "key")
        var key: String

        @OptionalField(key: "verified_at")
        var verifiedAt: Date?

        @Timestamp(key: "created_at", on: .create)
        var createdAt: Date?

        @Timestamp(key: "updated_at", on: .update)
        var updatedAt: Date?

        @Parent(key: "user_id")
        var user: User.Record

        init() {}
    }
}

extension SymmetricKey: LosslessStringConvertible {
    public init?(_ description: String) {
        guard let data = description.data(using: .utf8) else {
            return nil
        }

        self.init(data: data)
    }

    public var description: String {
        withUnsafeBytes {
            String(decoding: Data(Array($0)), as: UTF8.self)
        }
    }

    init(_ key: String, size: SymmetricKeySize = .bits192) throws {
        guard let data = key.data(using: .utf8) else {
            throw CryptoKitError.incorrectParameterSize
        }

        let dataSize = size.bitCount / 8

        guard data.count == dataSize else {
            throw CryptoKitError.incorrectKeySize
        }

        self.init(data: data)
    }
}

extension OTP.Migrations {
    struct CreateOTP: AsyncMigration {
        func prepare(on database: Database) async throws {
            try await database.schema("otp_configs")
                .id()
                .field("key", .string, .required)
                .field("verified_at", .datetime)
                .field("created_at", .datetime)
                .field("updated_at", .datetime)
                .field("user_id", .uuid, .required)
                .foreignKey("user_id", references: "users", "id", name: "users_fk")
                .unique(on: "user_id")
                .create()
        }

        func revert(on database: Database) async throws {
            try await database.schema("otp_configs").delete()
        }
    }
}

Expected behavior

I did not expect this error to be shown at all.

Environment

Additional context

It is more likely to occur when I quickly call the same endpoint multiple times.

RussBaz commented 9 months ago

OK, it turns out that the key can indeed sometimes contain \0 sequence, and it breaks Postgres… Any recommendations on how to possibly store such a data?

gwynne commented 7 months ago

The classical approach would be Base64 encoding:

extension SymmetricKey: LosslessStringConvertible {
    public init?(_ description: String) {
        guard let data = Data(base64Encoded: description) else {
            return nil
        }
        self.init(data: data)
    }

    public var description: String {
        self.withUnsafeBytes { Data($0).base64EncodedString() }
    }

    init(_ key: String, size: SymmetricKeySize = .bits192) throws {
        guard let data = Data(base64Encoded: key) else {
            throw CryptoKitError.incorrectParameterSize
        }
        guard data.count size.bitCount >> 3 else {
            throw CryptoKitError.incorrectKeySize
        }
        self.init(data: data)
    }
}
gwynne commented 7 months ago

The alternative would be to store it directly as [UInt8] (using .array(of: .uint8) in the migration) instead of doing String conversions of any kind (which are relatively expensive even for simple hexadecimal representation, much less Base64).