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

PDF URLs #1142

Open
chestnutbak opened this issue Feb 7, 2023 · 7 comments · Fixed by invoice-crater/crater#6 · May be fixed by #1150
Open

PDF URLs #1142

chestnutbak opened this issue Feb 7, 2023 · 7 comments · Fixed by invoice-crater/crater#6 · May be fixed by #1150
Assignees
Labels
bug Something isn't working

Comments

@chestnutbak
Copy link

chestnutbak commented Feb 7, 2023

Describe the bug
We have an issue where the incorrect PDF gets loaded when trying to view specific invoices.
Here are the links to the different invoices:
https://example.com/invoices/pdf/7PVveOmgj7pqgrNLRG16
https://example.com/invoices/pdf/7PVveOmgj7PqgrNLRG16

Notice how there is only a capital P or p difference in the URL... I can only assume this is causing the issue because I can't see what else it might be.
When we check the local storage both PDFs are there with their unique numbers.
In this instance the invoice we are loading is number 709 but the one that shows up is 685.
Our numbering is: Receipt - 2022-23/00685

Also... This is not the first time this has happened but I cannot find the other numbers and URLs that it happened with as they were deleted from the system.

Expected behavior
The correct PDF gets loaded when viewing every invoice.

Screenshots
image

Please complete the following information:

  • Crater version: 6.0.6
  • PHP version:
  • Database type and version:
@pepa65
Copy link

pepa65 commented Mar 9, 2023

This is a real bug, that I think should not be hard to fix. Today it happened 3 times, and the only thing we can tell Accounting is to make the same invoice again (hoping this time the uppercase/lowercase id does not clash...).

Are the clashes happening so frequent because the first twenty or so characters of the receipt name are the same??

@pepa65
Copy link

pepa65 commented Mar 9, 2023

To clarify the bug:

  • The pdf gets generated correctly, and is stored with the correct name in /var/www/html/crater/storage/app/Invoices.
  • The correct unique_hash gets stored in the invoices table.
  • When clicking on the link (https://example.com/admin/invoices/878/view) in the list of invoices on the Invoices page, the wrong pdf gets shown in the pdf-viewer, if the unique_hash of the internal URL of the correct one has one matches the unique_hash of an older invoice (except for case).
  • These pdf URLs can be seen from the pdf window, when clicking on the ... and then select Copy PDF URL (https://example.com/invoices/pdf/8V1DGOqxnY1RxnjNmpYR)
  • If an older pdf has the URL with the unique_hash like 8v1dgoQXNy1rXNJnMPyr (case inverted) then it will always be shown in the pdf-viewer (even if only 1 case gets flipped).
  • When entering the URL https://example.com/invoices/pdf/8V1DGOqxnY1RxnjNmpYR in the browser's URL bar (or when doing a wget), the wrong pdf gets retrieved, if an older one with a matching unique_hash (except for case) exists.

The bug is where the pdf retrieval function (maybe in Vue?) ignores case, and the match then finds the oldest document that matches.

@mohitpanjwani
Copy link
Contributor

Thanks for the feedback & suggestion @chestnutbak & @pepa65.

We are using a package called laravel-hashids in order to generate unique ids for the invoices but we noticed just now that it has many repeating characters on the hashids.php file which is causing the issue.

Removing the repeating characters from the configuration should fix the problem. We will also create a new migration in order to update existing invoices with the same unique_hash to fix existing duplicates.

@radhika587 is working on creating a PR for this asap.

@mohitpanjwani mohitpanjwani added the bug Something isn't working label Mar 9, 2023
@pepa65
Copy link

pepa65 commented Mar 9, 2023

But how about the uppercase/lowercase issue? If you use this URL to download an invoice: https://example.com/invoices/pdf/8V1DGOqxnY1RxnjNmpYR you will get https://example.com/invoices/pdf/8V1DGOqxnY1RxnjNmpYr (see the final char) if it is the unique_hash of an earlier pdf.

You an easily confirm this bug by changing the hashid of an older invoice in the database to differ only in case from a newer invoice.

@radhika587 radhika587 linked a pull request Mar 9, 2023 that will close this issue
@pepa65
Copy link

pepa65 commented Mar 20, 2023

@radhika587 If I patch this by hand, is there anything else I need to do with composer/artisan to make sure it's being applied?

@mohitpanjwani
Copy link
Contributor

@pepa65 No need to do anything except patching these changes.. but please note that it will not fix or update the old invoices/estimates/payments that are already created.

In order to fix that, we will need to create a separate one-off command or something that will run a query for the duplicate records and regenerate the hash ids for it.

@pepa65
Copy link

pepa65 commented Mar 21, 2023

In our case, the accounting department had to issue the invoice, customer waiting, so they generated another one. The "bad" ones just got deleted...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants