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.image() doesn't allow file names starting with 'data' #794

Closed
1nv opened this issue May 30, 2023 · 6 comments
Closed

pdf.image() doesn't allow file names starting with 'data' #794

1nv opened this issue May 30, 2023 · 6 comments

Comments

@1nv
Copy link

1nv commented May 30, 2023

Error details

When trying to call pdf.image() with filename starting with 'data', an error occurs:

imageData = base64Image.split("base64,")[1]
IndexError: list index out of range

This is caused by treating anything starting with 'data' as base64 string (see load_image() with elif filename.startswith("data"): check), but some file paths might also start with those characters.

Minimal code

from fpdf import FPDF

pdf = FPDF()
pdf.image('data/image.png', x=0, y=0, w=10, h=10)

Environment

  • Operating System: Windows 10
  • Python version: 3.10.11
  • fpdf2 version used: 2.7.4
@1nv 1nv added the bug label May 30, 2023
@Lucas-C
Copy link
Member

Lucas-C commented Jun 1, 2023

Hi @1nv and thank you for reporting this.

I started taking a look at this in #796

Would it be OK for you if we checked for the data: prefix instead of just data?

@Lucas-C Lucas-C added the image label Jun 1, 2023
@Lucas-C
Copy link
Member

Lucas-C commented Jun 1, 2023

@allcontributors please add @1nv for bug

@allcontributors
Copy link

@Lucas-C

I've put up a pull request to add @1nv! 🎉

@1nv
Copy link
Author

1nv commented Jun 1, 2023

Hi @Lucas-C,

Would it be OK for you if we checked for the data: prefix instead of just data?

Yes, that's exactly what I was thinking of, since : is not a valid character for paths (except for drive letters on Windows and mount points on one other platform).

EDIT: I just checked, and it is still valid on Unix/Linux. But this should still be fine, I think.

@Lucas-C Lucas-C closed this as completed in 6779a1f Jun 1, 2023
@Lucas-C
Copy link
Member

Lucas-C commented Jun 1, 2023

Yes, that's exactly what I was thinking of, since : is not a valid character for paths (except for drive letters on Windows and mount points on one other platform).

Thanks for your feedback

I merged #796

It is not released yet, but you can install the latest version of fpdf2 straight from git this way:

pip install git+https://github.com/PyFPDF/fpdf2.git@master

If you can test it @1nv I'd be happy to know if that solved your issue 😊

@Lucas-C Lucas-C reopened this Jun 1, 2023
@1nv
Copy link
Author

1nv commented Jun 1, 2023

I just tested the way you said, and it works fine!
Thank you for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants