Conversation
POST /versionsource returned a silent 500 for legacy imports whose
source object was stored with ContentType: application/octet-stream.
shouldCreateVersion gates only text/html and application/json, so even
when an explicit label was supplied the version write was skipped and
postObjectVersionWithLabel returned { error: 'Version was not created' }.
The diagnostic added in #284 confirmed 9 occurrences/24h with an
identical fingerprint (octet-stream, hadLabel, currentStatus=200).
When the caller passes an explicit label, treat the version as
requested-by-name and create it regardless of contentType. Auto-version
on plain PUT still gates to html/json, so storage cost is bounded to
the labelled call rate.
Refs: COR-55, COR-46
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Project convention: ticket IDs belong in commit messages and PR descriptions, not source code (they rot as tickets are renumbered or deleted). Co-Authored-By: Paperclip <noreply@paperclip.ing>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…e gate Pivot from the gate-widening approach (createVersion || label != null) to healing the underlying metadata on the labelled-version path. postObjectVersionWithLabel now derives a versionable mime from daCtx.ext when the stored ContentType is missing or application/octet-stream: html -> text/html json -> application/json The inferred type is passed via update.type. shouldCreateVersion sees the healed type, the version snapshot stores ContentType: text/html (or json), and the main object's PUT overwrites the stale ContentType in S3 metadata so the file is self-healed for all future requests. Binary files (jpg/pdf/etc.) still cannot be labelled-versioned, matching the project's "binaries do not version" semantics. The diagnostic log from #284 is retained and extended with inferredType + ext, so the unhealed path is observable. Tests updated: - new: legacy octet-stream HTML labelled version heals snapshot + main - new: labelled version on non-versionable ext still 500s with diagnostic - companion: plain PUT auto-version gate intact (no leak from labelled path) Co-Authored-By: Paperclip <noreply@paperclip.ing>
adobe-bot
pushed a commit
that referenced
this pull request
May 29, 2026
Collaborator
|
🎉 This PR is included in version 1.9.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary (revised after board pushback)
POST /versionsourcereturned a silent 500 for legacy imports whose source object was stored withContentType: application/octet-stream(or no ContentType). The diagnostic from #284 confirmed 9/24h occurrences with that identical fingerprint.Fix (was A, now B)
Original proposal (A) was to widen the gate:
createVersion = shouldCreateVersion(contentType) || update.label != null. Board flagged that we should fix the underlying ContentType rather than let bad metadata through.This PR ships fix B: in
postObjectVersionWithLabel, infer the correct mime fromdaCtx.extwhen the stored ContentType is missing or octet-stream, then pass that inferred type throughupdate.type.Effects:
text/html.inferredType+ext, so any future unhealed pattern is still observable in Cloudflare Logs.Tests
heals legacy octet-stream HTML on labelled version: snapshot + main object both repaired- asserts the version snapshot hasContentType: text/html, the main object PUT writesContentType: text/html, the audit entry is emitted with versionLabel + versionId.logs diagnostics and returns 500 when labelled version requested on non-versionable ext- asserts the silent-500 path still 500s for unknown extensions, and the diagnostic log captures contentType / inferredType / ext / hadLabel / currentStatus.plain PUT (no label) still skips auto-version for non-html/json contentType- companion to confirm the labelled-path mime inference does not bleed into auto-versioning.Risk
Refs