Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limiting file size #105

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Limiting file size #105

wants to merge 12 commits into from

Conversation

SoujitD-SAP
Copy link
Contributor

Limit the upload size of an attachment in the attachment plugin. Currently the maximum size size of data that can be scanned by the Malware Scanning Service on BTP is 400MB. So we would limit the file upload size to 400 MB.

  1. Handling PUT request with 'before' handler to fetch the content length of the uploaded attachment in plugin.js.

  2. Impose validation checks for size limit and rejecting the request which are above size limit.

  3. Currently cannot access the UI part but validation checks are both imposed for hybrid and local mode.

lib/plugin.js Outdated Show resolved Hide resolved
lib/plugin.js Outdated Show resolved Hide resolved
@SoujitD-SAP SoujitD-SAP removed the request for review from jeevitha011 October 22, 2024 07:35
@muskansethi1 muskansethi1 changed the title Making the inital commit Limiting file size Oct 24, 2024
CHANGELOG.md Outdated
@@ -4,6 +4,12 @@ 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 1.1.9
Copy link
Contributor

Choose a reason for hiding this comment

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

Changelog has to be added in 1.1.8 version itself

CHANGELOG.md Outdated

### Added

- Size validation check for uploaded attachments.
Copy link
Contributor

Choose a reason for hiding this comment

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

Added limit for file size ....

lib/plugin.js Outdated
const scanEnabled = cds.env.requires?.attachments?.scan ?? true
if(scanEnabled && status !== 'Clean') {
req.reject(403, 'Unable to download the attachment as scan status is not clean.');
const status = await AttachmentsSrv.getStatus(req.target, {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the code formatting

lib/plugin.js Outdated
}
}
}

async function readAttachment([attachment], req) {
if (!req?.req?.url?.endsWith("/content") || !attachment || attachment?.content) return;
let keys = { ID : req.req.url.match(attachmentIDRegex)[1]};
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

here also

@muskansethi1
Copy link
Contributor

muskansethi1 commented Oct 24, 2024

You can edit the script in package.json - "test": "npx jest attachments.test.js" to include unit testing in the PR checks

@SoujitD-SAP
Copy link
Contributor Author

Changes in CHANGELOG.md with mention of UI5 version compatibilty. Tested in local and hybrid mode.

lib/plugin.js Outdated
@@ -6,7 +6,7 @@ const attachmentIDRegex = /\/\w+\(.*ID=([0-9a-fA-F-]{36})/;

cds.on("loaded", function unfoldModel(csn) {
if (!("Attachments" in csn.definitions)) return;
const csnCopy = structuredClone(csn)
const csnCopy = structuredClone(csn);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove all the formatting changes in this file, as these lines are out of scope for this specific task.

@SoujitD-SAP
Copy link
Contributor Author

Formatted by tallying from main branch

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.

3 participants