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

Crash on import pkpass without pass data #2317

Open
brainrom opened this issue Jan 25, 2025 · 2 comments
Open

Crash on import pkpass without pass data #2317

brainrom opened this issue Jan 25, 2025 · 2 comments
Labels
common: frequent Affects or can be seen by most users regularly or impacts most users' first experience severity: major Severely degrades major functionality or product features, with no satisfactory workaround type: bug Something isn't working

Comments

@brainrom
Copy link

Version: 2.34.4 from F-Droid

Catima crashed when I tried to import loyalty card from pkpass file.

Crash happens because of this unhandled exception

throw FormatException(mContext.getString(R.string.errorReadingFile))

Pkpass file may not contain one of "boardingPass", "coupon", "eventTicket", "generic" fields. Manually adding empty "generic" field to the pkpass, solved the problem with certain pkpass and it successfully imported, because import of passdata just inserts text in card description/note field and nothing more.

Maybe change this confition to !hasPassData || !pkPassHasBarcodes and then add exception handler with meaningful message.

Thank you.

@TheLastProject
Copy link
Member

That exception is not supposed to be unhandled, oops! That's definitely the main bug here: it shouldn't crash, that message should be displayed to the user.

The pkpass documentation states that the type shows which style a pkpass file should be displayed with, which Catima doesn't do anything with yet. I can't really find documentation if one of those fields is required right now, so maybe we can just assume a "generic" style in the future if none of them is defined.

So, to fix:

  1. Properly bubble up the exception so it shows to the user
  2. Allow Catima to deal with none of the fields existing (your suggested fix sound good in principle, would have to double-check when coding)

@TheLastProject TheLastProject added type: bug Something isn't working severity: major Severely degrades major functionality or product features, with no satisfactory workaround common: occasional Affects or can be seen by some users regularly or most users rarely common: frequent Affects or can be seen by most users regularly or impacts most users' first experience and removed common: occasional Affects or can be seen by some users regularly or most users rarely labels Jan 25, 2025
@brainrom
Copy link
Author

Thank you for quick response!
Will you fix it? I'm not an Android developer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common: frequent Affects or can be seen by most users regularly or impacts most users' first experience severity: major Severely degrades major functionality or product features, with no satisfactory workaround type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants