Skip to content

Adding support to keep original Filename of Attachment and open more flexibility#645

Closed
fglueck wants to merge 17 commits into
barbushin:masterfrom
fglueck:master
Closed

Adding support to keep original Filename of Attachment and open more flexibility#645
fglueck wants to merge 17 commits into
barbushin:masterfrom
fglueck:master

Conversation

@fglueck

@fglueck fglueck commented Dec 13, 2021

Copy link
Copy Markdown

can be extend with settings like "OVERWRITE_EXISTING _FILES" or like "GENERATE_FILENAME_IF_FILE_EXISTS"...

fglueck added 5 commits April 10, 2021 13:04
Adding support for "original" Filename.
$mailbox->setAttachmentsFilenameMode(Mailbox::ATTACH_FILE_NAMERANDOM); # filename by random (old way)
$mailbox->setAttachmentsFilenameMode(Mailbox::ATTACH_FILE_NAMEORIGINAL); # filename by email
@Sebbo94BY Sebbo94BY changed the base branch from master to develop December 13, 2021 22:21
@Sebbo94BY

Copy link
Copy Markdown
Collaborator

@fglueck can you please resolve the conflicts?

@Sebbo94BY

Copy link
Copy Markdown
Collaborator

@fglueck friendly reminder :)

@Sebbo94BY Sebbo94BY changed the base branch from develop to master December 28, 2021 00:33
@Sebbo94BY

Copy link
Copy Markdown
Collaborator

Can you please also squash your commits at the end, so that it's only one?

It would be also cool, when the commit message would be Adding support for "original" Filename instead of Update Mailbox.php.

Thank you! :)

@fglueck

fglueck commented Jan 10, 2022

Copy link
Copy Markdown
Author

sorry I miss the button "Squash and merge".

Is it possible that you must first "allow" this?
https://github.blog/2016-04-01-squash-your-commits/

@Sebbo94BY

Copy link
Copy Markdown
Collaborator

I have unfortunately no permissions to change any settings of this project.

But you can usually rebase and squash locally and force push the changes, don't you?

Ok, if it's not possible, I also can squash and merge it through the buttons here. :)

@Sebbo94BY

Copy link
Copy Markdown
Collaborator

Wouldn't it be a bit more beautiful and more human readable, when there would be some more underscores in the constant names? Like this?

    public const ATTACH_FILE_NAME_RANDOM = 1; // Filename is unique (random)
    public const ATTACH_FILE_NAME_ORIGINAL = 2; // Filename is Attachment-Filename
    public const ATTACH_FILE_NAME_ITTERATED = 3; // Filename is Attachment-Filename but if allready exists it will be extend by Number (#nr)

@fglueck

fglueck commented Jan 14, 2022

Copy link
Copy Markdown
Author

Your right, I change the names and add itterated filename.

@Sebbo94BY

Copy link
Copy Markdown
Collaborator

Perfect, I've already seen it.

There are some more reviews (see above). :)

@fglueck

fglueck commented Jan 17, 2022

Copy link
Copy Markdown
Author

You meen the "squash" ? I never do this before... so I can try to do this...

@Sebbo94BY

Copy link
Copy Markdown
Collaborator

No, I mean these pending reviews in the code.

Adding support for "original" Filename.
$mailbox->setAttachmentsFilenameMode(Mailbox::ATTACH_FILE_NAME_RANDOM); # filename by random (old way)
$mailbox->setAttachmentsFilenameMode(Mailbox::ATTACH_FILE_NAME_ORIGINAL); # filename by email

@Sebbo94BY Sebbo94BY left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, forgot to publish my review. 😞

Comment thread src/PhpImap/Mailbox.php Outdated
public const ATTACH_FILE_NAMERANDOM = 1; // Filename is unique (random)
public const ATTACH_FILE_NAMEORIGINAL = 2; // Filename is Attachment-Filename
/** @var int */
protected $attachmentsFilenameMode = self::ATTACH_FILE_NAMERANDOM;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected $attachmentsFilenameMode = self::ATTACH_FILE_NAMERANDOM;
protected $attachmentsFilenameMode = self::ATTACH_FILE_NAMERANDOM;

Comment thread src/PhpImap/Mailbox.php

public const ATTACH_FILE_NAMERANDOM = 1; // Filename is unique (random)
public const ATTACH_FILE_NAMEORIGINAL = 2; // Filename is Attachment-Filename
/** @var int */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/** @var int */
/** @var int */

Comment thread src/PhpImap/Mailbox.php Outdated
/**
* Set $this->setAttachmentsRandomFilename param.
*
* @param int $random ATTACH_FILE_NAMERANDOM, ATTACH_FILE_NAMEORIGINAL

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable in the comment should match the actual parameter variable.

Suggested change
* @param int $random ATTACH_FILE_NAMERANDOM, ATTACH_FILE_NAMEORIGINAL
* @param int $mode ATTACH_FILE_NAMERANDOM, ATTACH_FILE_NAMEORIGINAL

Comment thread src/PhpImap/Mailbox.php
public function setAttachmentsFilenameMode(int $mode) : Mailbox
{
$this->attachmentsFilenameMode = $mode;
return $this;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this return here? Wouldn't be a void function enough?

Comment thread src/PhpImap/Mailbox.php
*/
public function setAttachmentsFilenameMode(int $mode) : Mailbox
{
$this->attachmentsFilenameMode = $mode;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I miss a check here, which ensures, that only supported modes can be set. Something like this:

Suggested change
$this->attachmentsFilenameMode = $mode;
if (!defined($mode))
{
throw new UnexpectedValueException("The defined mode '$mode' is not supported. Supported modes: ATTACH_FILE_NAMERANDOM, ATTACH_FILE_NAMEORIGINAL");
}
$this->attachmentsFilenameMode = $mode;

Note: I haven't tested this code.

Sebbo94BY added a commit that referenced this pull request May 10, 2026
The existing boolean `setAttachmentFilenameMode(true|false)` stays intact for
backward compatibility, and attachment downloads now either overwrite as
before or generate `name (1).ext`, `name (2).ext`, while still sanitizing path
separators and respecting the file-path limit (`src/PhpImap/Mailbox.php:1532`,
`src/PhpImap/Mailbox.php:1827`).

Co-authored by @fglueck

Refs:
- #645
@Sebbo94BY

Copy link
Copy Markdown
Collaborator

Thanks for your contribution!

I've rebased your changes based on the current code base.

This feature has been partitially already implemented as part of #751. The rest has been implemented as part of #751.

See the above linked pull requests for more details.

@Sebbo94BY Sebbo94BY closed this May 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants