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

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] 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 https://github.com/Expensify/App/pull/32562

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Tap fab --- Request money ---Scan
3. Upload a corrupt pdf
4. Tap request
5. Note no error displayed for uploading corrupt pdf
6. Tap on created scan request
7. 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?

• [ ] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/ced0a150-a83f-4cdf-b11f-71cce6e66f35

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-03-07

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[codebycarlos]

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.

https://github.com/Expensify/App/blob/393f2bec9a6a1d9f3affa18207e6b735274d1c2f/src/pages/iou/ReceiptSelector/index.js#L102-L120

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..

[FlorianWueest]

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: https://github.com/Expensify/App/blob/fa533cea7d1b3c177ef698a4de6f90fe4a5c3362/src/pages/iou/ReceiptSelector/index.js#L102-L120

With those parameters: https://github.com/Expensify/App/blob/fa533cea7d1b3c177ef698a4de6f90fe4a5c3362/src/CONST.ts#L51-L60

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.

[samilabud]

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:

https://github.com/Expensify/App/blob/0c351a4534f0bfb8278f2fa9b8c0a10187b6c8bb/src/components/PDFView/index.js#L6

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:

https://github.com/Expensify/App/assets/5216036/66335445-2983-43c0-b3e2-21ab24184b25

[Christinadobrzyn]

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

[situchan]

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

yes, it's a dupe

[ishpaul777]

Proposal

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

Request Money -Scan - Scan report allows protected PDF upload

What is the root cause of that problem?

We're not checking the PDF is weather the pdf is valid or not before uploadin it, and when BE scans the pdf for further processing it throws error.

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

We can add a validation on FE to verify if the pdf file is valid or not using the pdfjs fromreact-pdf

const isPdf = pdfjs.isPdfFile(file.name);
if (isPdf) {
    pdfjs
        .getDocument(fileSource)
        .promise.then(onSuccess)
        .catch(onError);
} else {
    onSuccess();
}

☝️ here onSucess is the code in below LOCs, and we need to create onError which should call setUploadReceiptError with valid translation keys.

https://github.com/Expensify/App/blob/a4bdebe94750e438fe22a7f569e68b54d37df1d0/src/pages/iou/request/step/IOURequestStepScan/index.js#L121-L144

For Native:

For native devices, We will try to render the PDF with zero visibilty using PDF component from react-native-pdf if the load complete we fire onSucess, i.e the image upload function else it throws error. (error copy needs to be decided)

// fires when when file is picked by user
const setReceiptAndNavigate = (file) => {
        if (!validateReceipt(file)) {
            return;
        }
        const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', ''));
        if (fileExtension.toLowerCase() === 'pdf') {
            setPdfFile(file);
            return;
        }
        onLoadComplete(file);
};

// fires when when pdf is rendered successfully by pdf 
const onLoadComplete = (file) => {
        // Store the receipt on the transaction object in Onyx
        IOU.setMoneyRequestReceipt(transactionID, file.uri, file.name, action !== CONST.IOU.ACTION.EDIT);

        if (action === CONST.IOU.ACTION.EDIT) {
            updateScanAndNavigate(file, file.uri);
            return;
        }

        navigateToConfirmationStep();
        setPdfFile(null);
};

// this renders with hidden visibilty
{pdfFile && (
                    <Pdf
                        source={{uri: pdfFile.uri, cache: true}}
                        onLoadComplete={onLoadComplete}
                        onError={() => {
                            Alert.alert(translate('receipt.pdfCorrupted'), translate('receipt.pdfCorruptedDescription'));
                            setPdfFile(null);
                        }}
                        style={styles.invisible}
                    />
)}

[sonialiap]

@situchan what do you think of the above proposals?

[melvin-bot[bot]]

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

[situchan]

proposals in review

[sonialiap]

@situchan any update on your review? :)

[situchan]

Everyone proposed solution for web only. This happens on native as well. Still awaiting proposals.

https://github.com/Expensify/App/assets/108292595/69f40913-c2f3-4eba-bc38-2e3a1171c90c

[situchan]

@sonialiap can we clarify the expected behavior here?

Should we prevent uploading corrupted pdf in advance?

[sonialiap]

Can we identify a corrupted pdf when the user attempts to upload it? If yes, I think that would be the best time to flag it so that the user doesn't have to wait for us to process the pdf and then tell them it didn't work.

[ishpaul777]

Proposal Updated

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@sonialiap @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[sonialiap]

@situchan what do you think of the updated proposal? https://github.com/Expensify/App/issues/34032#issuecomment-1898598514

[melvin-bot[bot]]

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

[situchan]

Thanks. I will evaluate them. Eta early next week

[sonialiap]

@situchan checking in on the proposals, how are they looking?

[melvin-bot[bot]]

@sonialiap @situchan this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

Current assignee @situchan is eligible for the Internal assigner, not assigning anyone new.

[situchan]

Thanks for proposals everyone. Do you mind sharing testing branches? Hard to follow your solutions suggested in proposals.

[melvin-bot[bot]]

@sonialiap, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@sonialiap, @situchan Eep! 4 days overdue now. Issues have feelings too...