Skip to content

Fix non-portable BLOB columnDefinition in WebAuthnCredential#275

Merged
devondragon merged 2 commits intomainfrom
fix/274-webauthn-blob-column-definition
Mar 12, 2026
Merged

Fix non-portable BLOB columnDefinition in WebAuthnCredential#275
devondragon merged 2 commits intomainfrom
fix/274-webauthn-blob-column-definition

Conversation

@devondragon
Copy link
Owner

@devondragon devondragon commented Mar 12, 2026

Summary

  • Remove hardcoded columnDefinition = "BLOB" from three byte[] fields in WebAuthnCredential (publicKey, attestationObject, attestationClientDataJson)
  • Remove @Lob annotations — plain byte[] with explicit length hints lets Hibernate pick the right native type per dialect without the inconsistent @Lob semantics across Hibernate versions
  • Set length = 65535 (not 65536) for attestationObject and attestationClientDataJson to stay within MySQL's BLOB threshold and avoid unexpected MEDIUMBLOB promotion
  • publicKey uses length = 2048 which fits comfortably in a standard BLOB

Fixes #274

Test plan

  • Full build and test suite passes (H2 in-memory)
  • Verify DDL generation on PostgreSQL (reporter can confirm)
  • Verify no regression on MySQL

Remove hardcoded `columnDefinition = "BLOB"` from three byte[] fields
in WebAuthnCredential. The `@Lob` annotation already handles dialect-
appropriate type mapping (BLOB for MySQL, bytea for PostgreSQL, etc.).
The explicit columnDefinition bypassed Hibernate's dialect translation,
causing DDL failures on PostgreSQL and other non-MySQL databases.

Fixes #274
Copilot AI review requested due to automatic review settings March 12, 2026 00:37
@claude
Copy link

claude bot commented Mar 12, 2026

Code Review

This is a correct and well-scoped fix. Removing the hardcoded columnDefinition = "BLOB" is the right approach — that attribute bypasses Hibernate's SQL dialect translation entirely, injecting raw MySQL DDL syntax that fails on PostgreSQL and other databases that lack a native BLOB type.

What the fix does well

  • Minimal and targeted — only the three problematic attributes are removed; nothing else changes
  • Root cause addressed — the @Lob annotation was already doing the right thing; the columnDefinition override was actively fighting it
  • Portable by default — letting Hibernate choose the storage type per dialect is the correct library design, especially since consuming apps bring their own database

Things worth verifying before merge

1. PostgreSQL: @Lob on byte[] maps to oid, not bytea

In Hibernate 6 (used by Spring Boot 3.5+/4.0+), @Lob on a byte[] field maps to PostgreSQL's Large Object (oid) type, not bytea. This is different from bytea in several ways:

  • oid uses a separate pg_largeobject system table and requires explicit transaction management
  • bytea is stored inline and is simpler to work with

The original reporter (#274) should verify that this actually works end-to-end on PostgreSQL, not just that DDL generation succeeds. If bytea is the intended target, @JdbcTypeCode(Types.VARBINARY) would be more explicit:

@JdbcTypeCode(Types.VARBINARY)
@Column(name = "public_key", nullable = false)
private byte[] publicKey;

That said, @Lob without columnDefinition is still a significant improvement over the current broken state, so this PR is worth merging even if a follow-up is needed.

2. Existing MySQL users — schema drift

This library has no Flyway/Liquibase migrations, so consumers using spring.jpa.hibernate.ddl-auto=update may see Hibernate attempt a column type change from BLOB to LONGBLOB (the default Hibernate 6 mapping for @Lob byte[] on MySQL). Most production deployments use validate or none, so this likely isn't a practical issue, but worth a note in the PR description or release notes.

3. attestationObject and attestationClientDataJson are nullable

This was true before the PR too, but worth confirming intentionally: attestationObject and attestationClientDataJson are nullable columns. That's fine if attestation data is optional in your WebAuthn flow, just confirming it's by design.


Test coverage

The H2 tests passing is a good baseline. Recommend the #274 reporter confirms:

  • DDL generates correctly on PostgreSQL (schema creation works)
  • Round-trip read/write of binary credential data works on PostgreSQL
  • No regression on MySQL (DDL and data round-trip)

Summary

Approved with notes. The fix is correct and directly addresses the reported issue. The Hibernate 6 + PostgreSQL oid vs bytea distinction is worth investigating as a potential follow-up, but removing columnDefinition = "BLOB" is unambiguously the right change here.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the JPA mapping for WebAuthn binary fields by removing explicit columnDefinition = "BLOB" declarations and relying on @Lob + the configured Hibernate dialect to choose the appropriate underlying column type.

Changes:

  • Removed explicit columnDefinition = "BLOB" from the public_key column mapping.
  • Removed explicit columnDefinition = "BLOB" from attestation_object and attestation_client_data_json column mappings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Replace `@Lob @column(columnDefinition = "BLOB")` with plain `byte[]`
and explicit `@Column(length = ...)` on the three binary fields in
WebAuthnCredential.

`@Lob` on byte[] in Hibernate 7 generates literal `blob` in DDL,
which bypasses dialect translation and fails on PostgreSQL (no `blob`
type). Plain byte[] lets each dialect choose the correct native type:
`bytea` on PostgreSQL, `varbinary`/`mediumblob` on MySQL.

Verified DDL generation against a real PostgreSQL 16 instance and
confirmed all three columns map to `bytea`. Full H2 test suite passes
with no regressions.

Fixes #274
@claude
Copy link

claude bot commented Mar 12, 2026

Code Review

The fix is correct in spirit — removing columnDefinition = \"BLOB\" resolves the PostgreSQL DDL failure. However, there are a few things worth noting about the actual diff vs. the PR description, plus one concrete concern.


PR description vs. actual diff

The summary says "The existing @Lob annotation already handles dialect-appropriate type mapping", but the diff actually removes @Lob as well (the import is dropped too). This is worth correcting in the description to avoid confusion.

Removing @Lob is arguably the better choice here: in Hibernate 6 (Spring Boot 3.5+/4.0+), @Lob on byte[] maps to PostgreSQL's oid (large object) type, not bytea. Large objects require explicit transaction management and use a separate system catalog (pg_largeobject). Without @Lob, Hibernate maps byte[] to bytea on PostgreSQL, which is simpler and more appropriate.

So the approach — byte[] + @Column(length=...), no @Lob — is actually more portable than what the description claims. The description should be updated to reflect this.


Concrete concern: length = 65536 exceeds MySQL VARBINARY / BLOB threshold

In Hibernate 6 with the MySQL dialect, byte[] column sizes are bucketed:

Length MySQL DDL type
≤ 255 TINYBLOB
≤ 65535 BLOB
≤ 16,777,215 MEDIUMBLOB

length = 65536 is 1 byte over the BLOB threshold, so Hibernate will generate MEDIUMBLOB for attestation_object and attestation_client_data_json.

Why this matters for existing MySQL users:

  • Users running with ddl-auto=update would see Hibernate attempt to alter BLOBMEDIUMBLOB, which may fail or cause unintended schema churn.
  • Most production setups use validate or none, so the practical impact is limited — but worth being intentional.

Recommendation: Change to length = 65535 to stay within the BLOB bucket on MySQL while still supporting up to 64 KB of attestation data. Attestation objects in practice are well under this limit.

// Before
@Column(name = \"attestation_object\", length = 65536)

// After
@Column(name = \"attestation_object\", length = 65535)
@Column(name = \"attestation_client_data_json\", length = 65535)

publicKey with length = 2048 looks correct

2048 bytes comfortably covers all COSE key types (EC2, RSA, OKP). Maps to BLOB on MySQL (> 255) and bytea on PostgreSQL. No concerns here.


Summary

Item Assessment
Remove columnDefinition = \"BLOB\" Correct fix
Remove @Lob Good — avoids PostgreSQL oid complications
length = 2048 for publicKey Fine
length = 65536 for attestation fields Minor issue — off-by-one past MySQL BLOB threshold; change to 65535
PR description accuracy Should be updated to reflect that @Lob is also removed

Overall this is a solid, necessary fix. The 65536 → 65535 tweak and a description update are the only suggested changes before merge.

@devondragon devondragon merged commit fc53d8b into main Mar 12, 2026
5 checks passed
@devondragon devondragon deleted the fix/274-webauthn-blob-column-definition branch March 12, 2026 01:03
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.

WebAuthnCredential.java uses non portable BLOB ColumnDefinition

2 participants