Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Fix AddressList toString method to quote semicolon #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Jul 6, 2019

Certain input of AddressList headers, cannot be converted to string and back to header object because of incorrect quoting:

From: "Foo;" <root@example.com>

gets incorrectly converted as

From: Foo; <root@example.com>

but ; is address separator, so it needs to be quoted:

From: "Foo;" <root@example.com>

The problem I discovered internally when using Storage\Message with Headers input, therefore testing that method is included in the unit test:

$message = new Message(['headers' => new Headers(), 'content' => (string)$body]);

// Mime\Decode::splitMessage calls toString on headers object which creates invalid result:
if ($message instanceof Headers) {
    $message = $message->toString();
}

@glensc glensc force-pushed the invalid-tostring branch from 8feb312 to c45b632 Compare July 6, 2019 02:26
@glensc
Copy link
Contributor Author

glensc commented Jul 6, 2019

@weierophinney, @Xerkus can you take care of this?

@glensc
Copy link
Contributor Author

glensc commented Jul 6, 2019

this is a regression from #147 as it added ; separator support that's why (i believe) this exception is caught.

however, there should not be an assumption that only zend-mail is used for reading writing mail messages.

@glensc glensc force-pushed the invalid-tostring branch from c45b632 to 3ee2a5e Compare July 6, 2019 02:31
glensc added a commit to eventum/zend-mail that referenced this pull request Jul 6, 2019
* pr/230: zendframework#230
  fix AddressList toString method to quote semicolon
glensc added a commit to glensc/eventum that referenced this pull request Jul 6, 2019
@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-mail; a new issue has been opened at laminas/laminas-mail#14.

@weierophinney
Copy link
Member

This repository has been moved to laminas/laminas-mail. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

  • Squash all commits in your branch (git rebase -i origin/{branch})
  • Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
  • Run the laminas/laminas-migration tool on the code.
  • Clone laminas/laminas-mail to another directory.
  • Copy the files from the second bullet point to the clone of laminas/laminas-mail.
  • In your clone of laminas/laminas-mail, commit the files, push to your fork, and open the new PR.
    We will be providing tooling via laminas/laminas-migration soon to help automate the process.

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

Successfully merging this pull request may close these issues.

2 participants