[LOW] [Splits] [$500] Mweb/Chrome - Scan request created using corrupt pdf displays incorrect error message

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 1.4.22
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #32562

Action Performed:

Go to https://staging.new.expensify.com/
Tap fab --- Request money ---Scan
Upload a corrupt pdf
Tap request
Note no error displayed for uploading corrupt pdf
Tap on created scan request
Enter all fields required

Expected Result:

Scan request created using corrupt pdf must display correct error message

Actual Result:

Scan request created using corrupt pdf displays incorrect error message " unexpected error while making request"

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

Bug6333288_1704467997273.income.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~019826bd315885912e
Upwork Job ID: 1743294656335773696
Last Price Increase: 2024-04-16

Job added to Upwork: https://www.upwork.com/jobs/~019826bd315885912e

@sonialiap

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@situchan

Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

I can't replicate this on my android mobile (I get the error post upload as expected). Could you please share the file?

I believe the receipt is validated here.

App/src/pages/iou/ReceiptSelector/index.js

Lines 102 to 120 in 393f2be

    
           function validateReceipt(file) { 
        
               const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', '')); 
        
               if (!CONST.API_ATTACHMENT_VALIDATIONS.ALLOWED_RECEIPT_EXTENSIONS.includes(fileExtension.toLowerCase())) { 
        
                   setUploadReceiptError(true, 'attachmentPicker.wrongFileType', 'attachmentPicker.notAllowedExtension'); 
        
                   return false; 
        
               } 
        
               if (lodashGet(file, 'size', 0) > CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE) { 
        
                   setUploadReceiptError(true, 'attachmentPicker.attachmentTooLarge', 'attachmentPicker.sizeExceeded'); 
        
                   return false; 
        
               } 
        
               if (lodashGet(file, 'size', 0) < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) { 
        
                   setUploadReceiptError(true, 'attachmentPicker.attachmentTooSmall', 'attachmentPicker.sizeNotMet'); 
        
                   return false; 
        
               } 
        
               return true; 
        
           }

It relies entirely on the metadata, without checking content. So differences in how the mobile operating system interprets the metadata could lead to these corrupted PDFs getting through..

Proposal

Please re-state the problem that we are trying to solve in this issue:

Corrupt pdfs can be uploaded as a receipt - and the error message is generic.

What is the root cause of that problem?

There's no specific pdf checking logic to determine whether the file is corrupted. Currently, we only check for size parameters and required extensions:

We check it here:

App/src/pages/iou/ReceiptSelector/index.js

Lines 102 to 120 in fa533ce

    
           function validateReceipt(file) { 
        
               const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', '')); 
        
               if (!CONST.API_ATTACHMENT_VALIDATIONS.ALLOWED_RECEIPT_EXTENSIONS.includes(fileExtension.toLowerCase())) { 
        
                   setUploadReceiptError(true, 'attachmentPicker.wrongFileType', 'attachmentPicker.notAllowedExtension'); 
        
                   return false; 
        
               } 
        
               if (lodashGet(file, 'size', 0) > CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE) { 
        
                   setUploadReceiptError(true, 'attachmentPicker.attachmentTooLarge', 'attachmentPicker.sizeExceeded'); 
        
                   return false; 
        
               } 
        
               if (lodashGet(file, 'size', 0) < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) { 
        
                   setUploadReceiptError(true, 'attachmentPicker.attachmentTooSmall', 'attachmentPicker.sizeNotMet'); 
        
                   return false; 
        
               } 
        
               return true; 
        
           }

With those parameters:

App/src/CONST.ts

Lines 51 to 60 in fa533ce

    
           API_ATTACHMENT_VALIDATIONS: { 
        
               // 24 megabytes in bytes, this is limit set on servers, do not update without wider internal discussion 
        
               MAX_SIZE: 25165824, 
        
               // An arbitrary size, but the same minimum as in the PHP layer 
        
               MIN_SIZE: 240, 
        
               // Allowed extensions for receipts 
        
               ALLOWED_RECEIPT_EXTENSIONS: ['jpg', 'jpeg', 'gif', 'png', 'pdf', 'htm', 'html', 'text', 'rtf', 'doc', 'tif', 'tiff', 'msword', 'zip', 'xml', 'message'], 
        
           },

What changes do you think we should make in order to solve the problem?

Either install front-end checks (render the pdf using 'react-pdf' and 'react-native-pdf'), to check for file corruption. Or adjust the genericEditFailureMessage to be more meaningful in the case of corrupted pdf file.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Mweb/Chrome - Scan request created using corrupt pdf displays incorrect error message

What is the root cause of that problem?

We are not checking if the PDF file is corrupted/valid file before uploading in the next components:
https://github.com/Expensify/App/blob/main/src/pages/iou/request/step/IOURequestStepScan/index.js
https://github.com/Expensify/App/blob/main/src/pages/iou/request/step/IOURequestStepScan/index.native.js

What changes do you think we should make in order to solve the problem?

We should validate the PDF before uploading the file in the previously mentioned files and show an error message if PDF is corrupted.

Based on our current PDF libraries:

App/src/components/PDFView/index.js

Line 6 in 0c351a4

import {Document, Page, pdfjs} from 'react-pdf/dist/esm/entry.webpack';

Create a function to check the integrity of the file:

import {pdfjs} from 'react-pdf/dist/esm/entry.webpack';

const checkPdfIntegrity = async (file, fileExtension) =>
    new Promise((resolve, reject) => {
        if (fileExtension !== CONST.IOU.FILE_TYPES.PDF) {
            return resolve({success: true});
        }
        const fileReader = new FileReader();
        fileReader.onload = async (event) => {
            const arrayBuffer = event.target && event.target.result;

            if (arrayBuffer) {
                try {
                    const loadingTask = pdfjs.getDocument(event.target.result);
                    const pdfDocument = await loadingTask.promise;
                    resolve({success: true, document: pdfDocument});
                } catch (error) {
                    resolve({success: false});
                }
            }
        };

        fileReader.onerror = (error) => {
            // eslint-disable-next-line prefer-promise-reject-errors
            reject({success: false, error});
        };

        fileReader.readAsArrayBuffer(file);
    });

Then inside of validateReceipt functions:

 const isValidPDF = await checkPdfIntegrity(file, fileExtension);

        if (!isValidPDF.success) {
            setUploadReceiptError(true, 'attachmentPicker.attachmentCorruptedError', 'attachmentPicker.pdfCorruptedError');
            return false;
        }

Also, we should add this last strings to languages files like: https://github.com/Expensify/App/blob/main/src/languages/en.ts
eg:

attachmentPicker: {
      .
      .
      .
        attachmentCorruptedError: 'Attachment is corrupted',
        pdfCorruptedError: 'The PDF file is corrupted',
    },

This could be the result:

Valid.PDF.Files.in.Scan.-.test.mp4

@situchan

Hey @situchan could you check if this GH is a duplicate - thanks!

Yes, I think tomorrow will be ready, today was reviewed internally, I will make adjustments and when approved internally I will open to external review

Hi I noticed on latest main that there is a bug that even when I am uploading pdf which is not corrupted I am getting error that file is corrupted and I think its related to this #40162 PR because of that I cant test newest changes

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

because of that I cant test newest changes

thank you for finding the cause. let's hold on #40471 (review)

@rushatgabhane

@rushatgabhane you shouldn't hold for #40471 – the mentioned issue was fixed in #40351, which is already merged.

@kubabutkiewicz please retry with the latest main.

Hi, just wanted to inform that I will be OOO from 29.04 to 05.05.

Issue not reproducible during KI retests. (First week)

Seems like #40471 (review) may have fixed the issue. Gonna close it since it's no longer reproducible. Please reopen if this is still happening.

Navigation Menu

Repository navigation

[LOW] [Splits] [$500] Mweb/Chrome - Scan request created using corrupt pdf displays incorrect error message #34032

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Bug0 Triage Checklist (Main S/O)

Proposal

Please re-state the problem that we are trying to solve in this issue:

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

135 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	function validateReceipt(file) {
	const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', ''));
	if (!CONST.API_ATTACHMENT_VALIDATIONS.ALLOWED_RECEIPT_EXTENSIONS.includes(fileExtension.toLowerCase())) {
	setUploadReceiptError(true, 'attachmentPicker.wrongFileType', 'attachmentPicker.notAllowedExtension');
	return false;
	}

	if (lodashGet(file, 'size', 0) > CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE) {
	setUploadReceiptError(true, 'attachmentPicker.attachmentTooLarge', 'attachmentPicker.sizeExceeded');
	return false;
	}

	if (lodashGet(file, 'size', 0) < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) {
	setUploadReceiptError(true, 'attachmentPicker.attachmentTooSmall', 'attachmentPicker.sizeNotMet');
	return false;
	}

	return true;
	}

	API_ATTACHMENT_VALIDATIONS: {
	// 24 megabytes in bytes, this is limit set on servers, do not update without wider internal discussion
	MAX_SIZE: 25165824,

	// An arbitrary size, but the same minimum as in the PHP layer
	MIN_SIZE: 240,

	// Allowed extensions for receipts
	ALLOWED_RECEIPT_EXTENSIONS: ['jpg', 'jpeg', 'gif', 'png', 'pdf', 'htm', 'html', 'text', 'rtf', 'doc', 'tif', 'tiff', 'msword', 'zip', 'xml', 'message'],
	},

[LOW] [Splits] [$500] Mweb/Chrome - Scan request created using corrupt pdf displays incorrect error message #34032

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Jan 5, 2024

melvin-bot commented on Jan 5, 2024

melvin-bot commented on Jan 5, 2024

Bug0 Triage Checklist (Main S/O)

melvin-bot commented on Jan 5, 2024

codebycarlos commented on Jan 5, 2024

FlorianWueest commented on Jan 5, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue:

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

samilabud commented on Jan 6, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

Christinadobrzyn commented on Jan 7, 2024

135 remaining items

kubabutkiewicz commented on Apr 15, 2024

kubabutkiewicz commented on Apr 16, 2024

melvin-bot commented on Apr 16, 2024

rushatgabhane commented on Apr 18, 2024

paultsimura commented on Apr 19, 2024

kubabutkiewicz commented on Apr 26, 2024

mvtglobally commented on May 1, 2024

luacmartins commented on May 1, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions