kootenpv / yagmail

Send email in Python conveniently for gmail using yagmail
MIT License
2.66k stars 265 forks source link

Attach a PDF file from temporary filestore with correct name #183

Open joaomcarlos opened 4 years ago

joaomcarlos commented 4 years ago

Hi,

I am getting PDF files stored in a filestore (minio) and sending them over email with Yagmail.

The flow is basically, download the file from minio into a temporary filestore location '/tmp/tmpwj2mv81c' and storing the metadata along side it (name, size, etc). Then attaching it to Yagmail for sending.

In the past (version ==0.11.224), I could add them like contents=body + [{temp_file_name: doc_name}] to the yag.send and they would get correctly sent.

However this "hack" no longer works.

Would it be possible to do this a different way? If not, could this feature be added to all filetypes instead of just images?

Thanks!

This would be what I would expect to do:

            yag = get_smtp_client()
            # do this after login to bypass Yagmail defaults
            yag.useralias = "Sender Alias"
            yag.user = "sender@alias.net"

            # wrap text in "raw" class to enforce Mime type
            body = [html]

            # resolve (download) Minio document to local FS documents
            with download_files(documents, 10) as files:
                resolved_documents = []
                for doc, temp_file_name in files.items():
                    doc_name = doc.split("/")[-1]  # last part is filename + extension
                    resolved_documents.append({temp_file_name: doc_name})

                result = yag.send(
                    to={email: name},
                    subject=subject,
                    contents=body,
                    attachments=resolved_documents,
                )

            if not result:
                raise EmailServiceError("Failed to send email.")

            logger.debug("Sent send_documents email.", extra=ctx)
kootenpv commented 4 years ago

Not sure how that hack worked or what changed unfortunately.

Maybe try attachments= instead of contents=?

Otherwise, might be possible to just:

 doc_name = "/tmp/" + doc.split("/")[-1]
 os.rename("/tmp/name", doc_name)
 resolved_documents.append(doc_name)

before sending

joaomcarlos commented 4 years ago

Hi,

How it worked?

If you set your pipfile.lock to version ==0.11.224, the following works flawlessly for generating an HTML email with properly named PDF and CSV attachments (this is the "hack"):

            yag = get_smtp_client()
            # do this after login to bypass Yagmail defaults
            yag.useralias = "Alias"
            yag.user = "alias@alias.net"

            # wrap text in "raw" class to enforce Mime type
            body = [html]

            # resolve (download) Minio document to local FS documents
            with download_files(documents, 10) as files:
                resolved_documents = []
                for doc, temp_file_name in files.items():
                    doc_name = doc.split("/")[-1]  # last part is filename + extension
                    resolved_documents.append({temp_file_name: doc_name})

                result = yag.send(
                    to={email: name},
                    subject=subject,
                    contents=body + resolved_documents,
                    newline_to_break=False,
                )

            if result == False:
                raise EmailServiceError("Failed to send email.")
joaomcarlos commented 4 years ago

Regarding your solution, I fear that might introduce a security problem :

Since the filename is the same for the global system (in this case, the container), all requests will share the same temporary filepath.

This would only work if:

Towards this, my with download_files(documents, min_file_size_sanity_check) function makes sure to use the mkstemp function (which is the secure alternative to make a temporary file that is unique and non-shared), and also with the with statement it makes sure to always delete those files afterward (this wouldnt affect uniqueness of the files though).

Is there a reason why the system_filename => email_filename mechanism is not supported for all file types?

kootenpv commented 4 years ago

There is probably no reason, just not a need earlier. It is a hairy part of the code so I don't feel at the moment comfortable to dive into this. But if you're willing to take the time to make a PR, that would surely make great addition.

joaomcarlos commented 3 years ago

Hi,

Forgot to add a reply.

I added this to the internal issue tracking on our project. When we have some more time we will try and come back to this.

Thanks :)