Skip to content
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).
The format is based on [Keep a Changelog](http://keepachangelog.com/).

## Version 3.13.2

### Fixed

- Multiple single attachments components now are visible in the UI with unique labels
- Upload of inline attachments with `@Core.AcceptableMediaTypes` annotation no longer crashes with a TypeError
- UI facets for inline attachments are now correctly added even when no `Attachments` composition is present in the application

## Version 3.13.1

### Fixed
Expand Down
6 changes: 6 additions & 0 deletions lib/generic-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ async function finalizePrepareAttachment(data, req, prefix) {
}
attachmentData.ID ??= cds.utils.uuid()

// On stream PUT, filename is not in req.data — fetch it from the DB record
if (prefix && !attachmentData.filename && req?.target && req?.params) {
const filename = await resolveFilename(req, prefix)
if (filename && filename !== "n/a") attachmentData.filename = filename
}
Comment thread
eric-pSAP marked this conversation as resolved.

let ext = attachmentData.filename
? extname(attachmentData.filename).toLowerCase().slice(1)
: null
Expand Down
14 changes: 10 additions & 4 deletions lib/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ const {
require("./csn-runtime-extension")
const LOG = cds.log("attachments")

cds.on(cds.version >= "8.6.0" ? "compile.to.edmx" : "loaded", unfoldModel)
cds.on(
cds.version.split(".")[0] >= 8 ? "compile.to.edmx" : "loaded",
unfoldModel,
)

// Register the db handler ONCE (not per-service) to intercept attachment INSERT
// and handle it through the attachments service instead of native DB insert
Expand Down Expand Up @@ -247,7 +250,11 @@ cds.once("served", () => {
*/
function unfoldModel(csn) {
const meta = (csn.meta ??= {})
if (!("sap.attachments.Attachments" in csn.definitions)) return
if (
!("sap.attachments.Attachments" in csn.definitions) &&
!("sap.attachments.Attachment" in csn.definitions)
)
return
if (meta._enhanced_for_attachments) return
// const csnCopy = structuredClone(csn) // REVISIT: Why did we add this cloning?
const hasFacetForComp = (name, facets) =>
Expand Down Expand Up @@ -598,11 +605,10 @@ cds.ApplicationService.handle_attachments = cds.service.impl(async function () {

if (!prefix) return next()

await onPrepareAttachment(req)
if (!validateAttachmentMimeType(req)) return
Comment on lines +608 to 609

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Logic Error: For stream PUT requests (when req.subject.ref.length > 1), req.data is the raw binary body, not an object with ${prefix}_content keys. validateAttachmentMimeType finds the inline prefix by checking req.data[${p}_content] — which is always falsy in this path — so it returns false, and the if (!validateAttachmentMimeType(req)) return guard then aborts the upload prematurely (a regression).

The original ordering called onPrepareAttachment after both validations, so this wasn't a problem before. In the stream PUT path, either the validation needs to be skipped when no content-bearing req.data key is present, or mimeType needs to be populated differently before validation runs.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

if (!(await validateAttachmentSize(req))) return
Comment thread
eric-pSAP marked this conversation as resolved.

await onPrepareAttachment(req)

if (getAttachmentKind() !== "db") {
await putInlineAttachmentObjectStore(req, prefix)
} else {
Expand Down
127 changes: 127 additions & 0 deletions tests/integration/attachments.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2922,6 +2922,67 @@ describe("Tests for single attachment entity", () => {
})
expect(Buffer.concat(chunks).length).toBeGreaterThan(0)
})

it("Should accept upload when file type matches @Core.AcceptableMediaTypes on inline attachment", async () => {
const svc = await cds.connect.to("ProcessorService")
const el = svc.entities.SingleAttachment.elements.myAttachment_content
const origTypes = el["@Core.AcceptableMediaTypes"]
el["@Core.AcceptableMediaTypes"] = ["application/pdf"]

try {
const { data: singleAttachment } = await POST(
"/odata/v4/processor/SingleAttachment",
{ name: "Mime type allowed test", myAttachment_filename: "sample.pdf" },
)

const filepath = join(__dirname, "content/sample.pdf")
const fileContent = readFileSync(filepath)

const putRes = await PUT(
`/odata/v4/processor/SingleAttachment(ID=${singleAttachment.ID},IsActiveEntity=false)/myAttachment_content`,
fileContent,
{ headers: { "Content-Type": "application/pdf" } },
)
expect(putRes.status).toEqual(204)
} finally {
el["@Core.AcceptableMediaTypes"] = origTypes
}
})

it("Should reject upload when file type does not match @Core.AcceptableMediaTypes on inline attachment", async () => {
const svc = await cds.connect.to("ProcessorService")
const el = svc.entities.SingleAttachment.elements.myAttachment_content
const origTypes = el["@Core.AcceptableMediaTypes"]
el["@Core.AcceptableMediaTypes"] = ["image/jpeg"]

try {
const { data: singleAttachment } = await POST(
"/odata/v4/processor/SingleAttachment",
{
name: "Mime type rejected test",
myAttachment_filename: "sample.pdf",
},
)

const filepath = join(__dirname, "content/sample.pdf")
const fileContent = readFileSync(filepath)

let expectedError
await PUT(
`/odata/v4/processor/SingleAttachment(ID=${singleAttachment.ID},IsActiveEntity=false)/myAttachment_content`,
fileContent,
{ headers: { "Content-Type": "application/pdf" } },
).catch((e) => {
expectedError = e
})
expect(expectedError?.response?.status).toEqual(400)
expect(expectedError?.response?.data?.error?.message).toMatch(
"The attachment file type 'application/pdf' is not allowed.",
)
} finally {
el["@Core.AcceptableMediaTypes"] = origTypes
}
})
})

describe("Tests for attachments facet disable", () => {
Expand Down Expand Up @@ -3014,6 +3075,72 @@ describe("Tests for attachments facet disable", () => {
expect(attachmentFacets.length).toEqual(1)
expect(attachmentFacets[0].Label).toEqual("My custom attachments")
})

it("Adds @UI.FieldGroup and @UI.Facet for an inline attachment when only sap.attachments.Attachment is used (no Attachments composition)", () => {
const model = cds.linked({
definitions: {
// Only the inline Attachment type — no Attachments composition aspect.
// Apps that never import `Attachments` so `sap.attachments.Attachments` is absent
// from the CSN, causing the early-return guard to wrongly skip all facet injection.
"sap.attachments.Attachment": {
kind: "type",
"@_is_media_data": true,
elements: {
content: { type: "cds.LargeBinary", "@_is_media_data": true },
filename: { type: "cds.String" },
status: { type: "cds.String" },
},
},
"TestSvc.MyEntity": {
kind: "entity",
elements: {
ID: { key: true, type: "cds.UUID" },
attachment_content: {
type: "cds.LargeBinary",
"@_is_media_data": true,
},
attachment_filename: { type: "cds.String" },
attachment_status: { type: "cds.String" },
},
"@UI.Facets": [],
},
},
})
cds.emit("compile.to.edmx", model)

const entity = model.definitions["TestSvc.MyEntity"]
expect(entity["@UI.FieldGroup#attachment"]).toBeDefined()
const inlineFacet = entity["@UI.Facets"].find(
(f) => f.Target === "@UI.FieldGroup#attachment",
)
expect(inlineFacet).toBeDefined()
expect(inlineFacet.$Type).toBe("UI.ReferenceFacet")
})

it("Does not add a facet when the entity has no inline attachment fields", () => {
const model = cds.linked({
definitions: {
"sap.attachments.Attachment": {
kind: "type",
"@_is_media_data": true,
elements: {
content: { type: "cds.LargeBinary", "@_is_media_data": true },
},
},
"TestSvc.MyEntity": {
kind: "entity",
elements: {
ID: { key: true, type: "cds.UUID" },
name: { type: "cds.String" },
},
"@UI.Facets": [],
},
},
})
cds.emit("compile.to.edmx", model)

expect(model.definitions["TestSvc.MyEntity"]["@UI.Facets"]).toHaveLength(0)
})
})

describe("Tests for acceptable media types", () => {
Expand Down
Loading