Skip to content
18 changes: 18 additions & 0 deletions lib/generic-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,24 @@ 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 dbKeys = Object.fromEntries(
Object.entries(req.params?.at(-1) || {}).filter(
([k]) => k !== "IsActiveEntity",
),
)
if (Object.keys(dbKeys).length > 0) {
const draftTarget = req.target.drafts || req.target
const record = await SELECT.one
.from(draftTarget, dbKeys)
.columns(`${prefix}_filename`)
if (record?.[`${prefix}_filename`]) {
attachmentData.filename = record[`${prefix}_filename`]
}
}
}
Comment thread
eric-pSAP marked this conversation as resolved.

let ext = attachmentData.filename
? extname(attachmentData.filename).toLowerCase().slice(1)
: null
Expand Down
9 changes: 6 additions & 3 deletions lib/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,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 +602,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
61 changes: 61 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
Loading