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.

Sign up for GitHub

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

fix: Add validation for gallery image upload #3457

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix: Add validation for gallery image upload #3457

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4f31c4
intial commit
Jan 29, 2025
d2f5d9a
new try
Feb 1, 2025
5a8b733
new try
Feb 1, 2025
353b768
artficat v4 upgrade
Feb 1, 2025
4d72ade
Merge branch 'master' into FYLE-86cxux3xk
sret-farhan2021 Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/app/shared/components/capture-receipt/capture-receipt.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,15 @@ export class CaptureReceiptComponent implements OnInit, OnDestroy, AfterViewInit
receiptsFromGallery$
.pipe(filter((receiptsFromGallery: string[]) => receiptsFromGallery.length > 0))
.subscribe((receiptsFromGallery) => {
const oversizedReceipts = receiptsFromGallery.filter((receiptBase64) => {
const receiptSize = this.calculateBase64Size(receiptBase64);
return receiptSize > 5 * 1024 * 1024; // 5MB in bytes
});

if (oversizedReceipts.length > 0) {
return this.showSizeLimitExceededPopover();
}

Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Mind it! Extract the size limit to a constant, partner!

The size limit of 5MB is hardcoded. Let's make it more maintainable by moving it to a constant at the top of the file.

+const MAX_RECEIPT_SIZE_MB = 5;
+const MAX_RECEIPT_SIZE_BYTES = MAX_RECEIPT_SIZE_MB * 1024 * 1024;

 const oversizedReceipts = receiptsFromGallery.filter((receiptBase64) => {
   const receiptSize = this.calculateBase64Size(receiptBase64);
-  return receiptSize > 5 * 1024 * 1024; // 5MB in bytes
+  return receiptSize > MAX_RECEIPT_SIZE_BYTES;
 });

Committable suggestion skipped: line range outside the PR's diff.

receiptsFromGallery.forEach((receiptBase64) => {
const receiptBase64Data = 'data:image/jpeg;base64,' + receiptBase64;
this.base64ImagesWithSource.push({
Expand All @@ -494,6 +503,27 @@ export class CaptureReceiptComponent implements OnInit, OnDestroy, AfterViewInit
.subscribe(() => this.setUpAndStartCamera());
}

calculateBase64Size(base64String: string): number {
const padding = (base64String.match(/(=+)$/) || [])[0]?.length || 0;
return (base64String.length * 3) / 4 - padding;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Magizhchi! Let's make this method bulletproof!

The base64 size calculation needs input validation and documentation. Here's what you need, boss!

+/**
+ * Calculates the size of a base64 string in bytes.
+ * @param base64String - The base64 encoded string
+ * @returns The size in bytes
+ * @throws Error if the input is not a valid base64 string
+ */
 calculateBase64Size(base64String: string): number {
+  if (!base64String || typeof base64String !== 'string') {
+    throw new Error('Invalid input: base64String must be a non-empty string');
+  }
+
+  // Basic validation of base64 format
+  if (!/^[A-Za-z0-9+/]*={0,2}$/.test(base64String)) {
+    throw new Error('Invalid input: not a valid base64 string');
+  }
+
   const padding = (base64String.match(/(=+)$/) || [])[0]?.length || 0;
   return (base64String.length * 3) / 4 - padding;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
calculateBase64Size(base64String: string): number {
const padding = (base64String.match(/(=+)$/) || [])[0]?.length || 0;
return (base64String.length * 3) / 4 - padding;
}
/**
* Calculates the size of a base64 string in bytes.
* @param base64String - The base64 encoded string
* @returns The size in bytes
* @throws Error if the input is not a valid base64 string
*/
calculateBase64Size(base64String: string): number {
if (!base64String || typeof base64String !== 'string') {
throw new Error('Invalid input: base64String must be a non-empty string');
}
// Basic validation of base64 format
if (!/^[A-Za-z0-9+/]*={0,2}$/.test(base64String)) {
throw new Error('Invalid input: not a valid base64 string');
}
const padding = (base64String.match(/(=+)$/) || [])[0]?.length || 0;
return (base64String.length * 3) / 4 - padding;
}


async showSizeLimitExceededPopover(): Promise<void> {
const sizeLimitExceededPopover = await this.popoverController.create({
component: PopupAlertComponent,
componentProps: {
title: 'Size limit exceeded',
message: 'The uploaded file is greater than 5MB in size. Please reduce the file size and try again.',
primaryCta: {
text: 'OK',
},
},
cssClass: 'pop-up-in-center',
});

await sizeLimitExceededPopover.present();
}
Comment on lines +508 to +522
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Style ah? Let's make this popover message dynamic!

The size limit is hardcoded in the message. Let's use our constant to make it dynamic and consistent!

 async showSizeLimitExceededPopover(): Promise<void> {
   const sizeLimitExceededPopover = await this.popoverController.create({
     component: PopupAlertComponent,
     componentProps: {
       title: 'Size limit exceeded',
-      message: 'The uploaded file is greater than 5MB in size. Please reduce the file size and try again.',
+      message: `The uploaded file is greater than ${MAX_RECEIPT_SIZE_MB}MB in size. Please reduce the file size and try again.`,
       primaryCta: {
         text: 'OK',
       },
     },
     cssClass: 'pop-up-in-center',
   });

Committable suggestion skipped: line range outside the PR's diff.


ngAfterViewInit(): void {
if (this.isModal) {
this.setUpAndStartCamera();
Expand Down
Loading