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

Fix mail messages with blank line on top #24

Merged
merged 3 commits into from
Sep 9, 2015
Merged

Fix mail messages with blank line on top #24

merged 3 commits into from
Sep 9, 2015

Conversation

Maks3w
Copy link
Member

@Maks3w Maks3w commented Sep 8, 2015

At the moment of write this patch GMail (a popular mail provider) prepend mail messages with a line with whitespaces

image

This PR remove that line before parse.

Add a data provider and compact assertions against current object state.
@Maks3w Maks3w added the WIP label Sep 8, 2015
@Maks3w Maks3w added this to the 2.5.2 milestone Sep 8, 2015
@@ -42,6 +42,8 @@ public function __construct(array $params)
} else {
$params['raw'] = stream_get_contents($params['file']);
}

$params['raw'] = ltrim($params['raw']);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should apply the workaround always? (out of the if block)

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense.

@Maks3w Maks3w removed the WIP label Sep 8, 2015
@Maks3w
Copy link
Member Author

Maks3w commented Sep 8, 2015

Reading issue #19 seems this is not enough and another alternative should be provided.

$this->assertEquals('Peter Müller <peter-mueller@example.com>', $message->from);
}

public function testGetHeaderAsArray()
Copy link
Member

Choose a reason for hiding this comment

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

Having these as separate tests ensures that we can have granular tests for each behavior. Pushing all assertions into a single method makes identifying breakages harder, as the first assertion that fails causes any following assertions to never execute, which means we often end up playing whack-a-mole (fix one assertion, only to find the same test fails on an additional assertion). Please re-instate these discrete methods.

Copy link
Member

Choose a reason for hiding this comment

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

I re-instated these when merging.

@weierophinney weierophinney self-assigned this Sep 9, 2015
@weierophinney
Copy link
Member

Reading issue #19 seems this is not enough and another alternative should be provided.

My reading of the two indicates they're completely separate issues.

@weierophinney weierophinney merged commit ef64587 into zendframework:master Sep 9, 2015
weierophinney added a commit that referenced this pull request Sep 9, 2015
Fix mail messages with blank line on top
weierophinney added a commit that referenced this pull request Sep 9, 2015
Re-instated each of:

- testIsMultipart()
- testGetHeader()
- testGetDecodedHeader()
- testGetHeaderAsArray()

each with the `filesProvider` data provider; these replace the newly added
`testParseFile()` in order to provide better granularity of assertions and
behavior.
weierophinney added a commit that referenced this pull request Sep 9, 2015
weierophinney added a commit that referenced this pull request Sep 9, 2015
weierophinney added a commit that referenced this pull request Sep 9, 2015
glensc pushed a commit to eventum/zend-mail that referenced this pull request Feb 15, 2016
This commit backports changes from the 2.5 series for the 2.4.8 LTS release.
These include the following:

- [zendframework#11](zendframework#11) ensures that the
  sender can be a mailbox name, or an email specifying a host that omits the
  TLD.
- [zendframework#21](zendframework#21) provides a patch to
  allow addresses in the form `Name address@domain.tld` (without angle brackets
  surrounding the actual email addres).
- [zendframework#24](zendframework#24) fixes parsing of
  mail messagse that contain an initial blank line (prior to any headers), a
  situation observed in particular with GMail.
- [zendframework#26](zendframework#26) fixes the
  `ContentType` header to properly handle parameters with encoded values.
weierophinney added a commit to zendframework/zendframework that referenced this pull request Dec 20, 2016
This commit backports changes from the 2.5 series for the 2.4.8 LTS release.
These include the following:

- [#11](zendframework/zend-mail#11) ensures that the
  sender can be a mailbox name, or an email specifying a host that omits the
  TLD.
- [#21](zendframework/zend-mail#21) provides a patch to
  allow addresses in the form `Name address@domain.tld` (without angle brackets
  surrounding the actual email addres).
- [#24](zendframework/zend-mail#24) fixes parsing of
  mail messagse that contain an initial blank line (prior to any headers), a
  situation observed in particular with GMail.
- [#26](zendframework/zend-mail#26) fixes the
  `ContentType` header to properly handle parameters with encoded values.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants