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

Storage\Maildir fixes + enable unit tests #82

Merged
merged 27 commits into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
962b1fb
Increase test coverage
fredden Mar 21, 2020
ef8b6c1
Fix broken tests
fredden Mar 21, 2020
4d72ca6
Fix bugs identified by testsuite
fredden Mar 21, 2020
4807afe
Increase test coverage
fredden Mar 21, 2020
3933c98
Increase test coverage
fredden Mar 21, 2020
b1686ed
Revert bugfixes as out-of-scope for this change
fredden Mar 24, 2020
5000780
Split get/set encoding tests
fredden Mar 26, 2020
7d17c8e
Split add & addFromString tests
fredden Mar 26, 2020
96cd866
Split get/remove parameter tests
fredden Mar 26, 2020
eed4b1a
Explain why invalid lines are invalid
fredden Mar 26, 2020
cd35f12
Assert exception messages
fredden Mar 26, 2020
bcd6212
Revert Maildir test changes
fredden Mar 26, 2020
ddf3d3a
Add exception message checking for maildir tests
fredden Mar 28, 2020
f0ebb63
Skip Maildir tests on Windows
fredden Mar 28, 2020
fe1ab49
Remove unnecessary property
fredden Mar 28, 2020
f2fbf32
Enable Maildir tests by default if possible
fredden Mar 28, 2020
8d80797
Fix broken tests
fredden Mar 28, 2020
8ae9b5d
Disable incomplete tests
fredden Mar 28, 2020
3cb4577
Enable functional tests
fredden Mar 28, 2020
88310bf
Add missing assertions
fredden Mar 28, 2020
41a3c57
Use expectException for permissions tests
fredden Mar 28, 2020
6864e64
Add missing hasFlags() test
fredden Mar 28, 2020
dca5d8c
Fix bugs identified by tests
fredden Mar 28, 2020
5e225cc
Move setSubject duplicate to its own test
fredden Mar 28, 2020
1770e14
Improve test condition
fredden Mar 28, 2020
c63504d
Ensure global variables are not changed by test
fredden Mar 28, 2020
6c6867c
Increase test coverage
fredden Mar 29, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Header/IdentificationField.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function getEncoding()
*/
public function toString()
{
return sprintf('%s: %s', $this->fieldName, $this->getFieldValue());
return sprintf('%s: %s', $this->getFieldName(), $this->getFieldValue());
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/Storage/Folder/Maildir.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ protected function buildFolderTree()
continue;
}

if ($this->_isMaildir($this->rootdir . $entry)) {
if ($this->isMaildir($this->rootdir . $entry)) {
fredden marked this conversation as resolved.
Show resolved Hide resolved
$dirs[] = $entry;
}
}
Expand Down Expand Up @@ -187,7 +187,7 @@ public function selectFolder($globalName)
$folder = $this->getFolders($this->currentFolder);

try {
$this->_openMaildir($this->rootdir . '.' . $folder->getGlobalName());
$this->openMaildir($this->rootdir . '.' . $folder->getGlobalName());
fredden marked this conversation as resolved.
Show resolved Hide resolved
} catch (Exception\ExceptionInterface $e) {
// check what went wrong
if (! $folder->isSelectable()) {
Expand Down
2 changes: 1 addition & 1 deletion src/Storage/Maildir.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public function getSize($id = null)
public function getMessage($id)
{
// TODO that's ugly, would be better to let the message class decide
if ($this->messageClass === Message\File::class
if (\trim($this->messageClass, '\\') === Message\File::class
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this change, please? Do we have any tests for that? It looks like a bugfix as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have tests: yes. They're disabled by default at present. I'm working on these separately. I'll move this bugfix to that branch.

Copy link
Member

Choose a reason for hiding this comment

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

@fredden before I start reviewing it again just a quick question: you were saying about extracting bugfixes into separate PR - is it still a plan? I can still see all changes here. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to move all the maildir test fixes to a separate pull request, but decided to leave here. If I move the bugfixes to a separate pull request then the testsuite will fail here until that lands. (The tests were being skipped by default, but run now.)

I'm happy to go with your recommendation. (Moving to a separate pull request isn't a lot of work.)

|| is_subclass_of($this->messageClass, Message\File::class)
) {
return new $this->messageClass([
Expand Down
14 changes: 13 additions & 1 deletion test/AddressListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function testIsEmptyByDefault()

public function testAddingEmailsIncreasesCount()
{
$this->list->add('test@example.com');
$this->list->addFromString('test@example.com');
fredden marked this conversation as resolved.
Show resolved Hide resolved
$this->assertEquals(1, count($this->list));
}

Expand All @@ -65,6 +65,18 @@ public function testGetReturnsFalseWhenEmailNotFound()
$this->assertFalse($this->list->get('foo@example.com'));
}

public function testThrowExceptionOnInvalidInputAdd()
{
$this->expectException('Laminas\Mail\Exception\InvalidArgumentException');
$this->list->add(null);
}

public function testThrowExceptionOnInvalidInputAddMany()
{
$this->expectException('Laminas\Mail\Exception\InvalidArgumentException');
$this->list->addMany([null]);
}

public function testGetReturnsAddressObjectWhenEmailFound()
{
$this->list->add('test@example.com');
Expand Down
26 changes: 26 additions & 0 deletions test/ConfigProviderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

/**
* @see https://github.com/laminas/laminas-mail for the canonical source repository
* @copyright https://github.com/laminas/laminas-mail/blob/master/COPYRIGHT.md
* @license https://github.com/laminas/laminas-mail/blob/master/LICENSE.md New BSD License
*/

namespace LaminasTest\Mail;

use Laminas\Mail\ConfigProvider;
use PHPUnit\Framework\TestCase;

/**
* @group Laminas_Mail
* @covers \Laminas\Mail\ConfigProvider<extended>
*/
class ConfigProviderTest extends TestCase
{
public function testInvoke()
{
$configProvider = new ConfigProvider();
$config = $configProvider();
$this->assertEquals(['dependencies'], \array_keys($config));
}
}
15 changes: 15 additions & 0 deletions test/Header/ContentTransferEncodingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public function testContentTransferEncodingGetFieldValueReturnsProperValue($enco
$contentTransferEncodingHeader = new ContentTransferEncoding();
$contentTransferEncodingHeader->setTransferEncoding($encoding);
$this->assertEquals($encoding, $contentTransferEncodingHeader->getFieldValue());
$this->assertEquals($encoding, $contentTransferEncodingHeader->getTransferEncoding());
}

/**
Expand Down Expand Up @@ -147,4 +148,18 @@ public function testSetTransferEncodingRaisesExceptionForInvalidValues()
$this->expectExceptionMessage('expects');
$header->setTransferEncoding("8bit\r\n 7bit");
}

public function testFromStringRaisesExceptionOnInvalidHeader()
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
ContentTransferEncoding::fromString('Foo: bar');
}

public function testEncodingAccessors()
fredden marked this conversation as resolved.
Show resolved Hide resolved
{
$header = new ContentTransferEncoding('today');
$this->assertEquals('ASCII', $header->getEncoding());
$header->setEncoding('UTF-8');
$this->assertEquals('ASCII', $header->getEncoding());
}
}
30 changes: 30 additions & 0 deletions test/Header/ContentTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,34 @@ public function invalidHeaderLinesProvider()
];
// @codingStandardsIgnoreEnd
}

public function testFromStringRaisesExceptionOnInvalidHeader()
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
ContentType::fromString('Foo: bar');
}

public function testEncodingAccessors()
fredden marked this conversation as resolved.
Show resolved Hide resolved
{
$header = new ContentType('today');
$this->assertEquals('ASCII', $header->getEncoding());
$header->setEncoding('UTF-8');
$this->assertEquals('UTF-8', $header->getEncoding());
}

public function testSetTypeThrowsOnInvalidValue()
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
fredden marked this conversation as resolved.
Show resolved Hide resolved
$header = new ContentType();
$header->setType('invalid');
}

public function testGetRemoveParameter()
{
$header = ContentType::fromString('content-type: text/plain; level=top');
$this->assertEquals('top', $header->getParameter('level'));
$this->assertEquals(true, $header->removeParameter('level'));
$this->assertEquals(false, $header->removeParameter('level'));
$this->assertEquals(null, $header->getParameter('level'));
fredden marked this conversation as resolved.
Show resolved Hide resolved
}
}
20 changes: 20 additions & 0 deletions test/Header/DateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,24 @@ public function testPreventsCRLFInjectionViaConstructor()
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
$address = new Header\Date("This\ris\r\na\nCRLF Attack");
}

public function testFromStringRaisesExceptionOnInvalidHeader()
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
Header\Date::fromString('Foo: bar');
}

public function testEncodingAccessors()
fredden marked this conversation as resolved.
Show resolved Hide resolved
{
$header = new Header\Date('today');
$this->assertEquals('ASCII', $header->getEncoding());
$header->setEncoding('UTF-8');
$this->assertEquals('ASCII', $header->getEncoding());
}

public function testToString()
{
$header = new Header\Date('today');
$this->assertEquals('Date: today', $header->toString());
}
}
24 changes: 20 additions & 4 deletions test/Header/GenericHeaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,30 @@
*/
class GenericHeaderTest extends TestCase
{
public function invalidHeaderLines()
{
return [
['Content-Type' . chr(32) . ': text/html; charset = "iso-8859-1"' . "\nThis is a test"],
['Content-Type: text/html; charset = "iso-8859-1"' . "\nThis is a test"],
fredden marked this conversation as resolved.
Show resolved Hide resolved
['Missing colon'],
fredden marked this conversation as resolved.
Show resolved Hide resolved
];
}
/**
* @dataProvider invalidHeaderLines
* @group ZF2015-04
*/
public function testSplitHeaderLineRaisesExceptionOnInvalidHeader()
public function testSplitHeaderLineRaisesExceptionOnInvalidHeader($line)
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
GenericHeader::splitHeaderLine(
'Content-Type' . chr(32) . ': text/html; charset = "iso-8859-1"' . "\nThis is a test"
);
GenericHeader::splitHeaderLine($line);
}

public function fieldNames()
{
return [
'append-chr-13' => ["Subject" . chr(13)],
'append-chr-127' => ["Subject" . chr(127)],
'non-string' => [null],
];
}

Expand Down Expand Up @@ -148,4 +156,12 @@ public function testAllowZeroInHeaderValueInConstructor()
$this->assertEquals(0, $header->getFieldValue());
$this->assertEquals('Foo: 0', $header->toString());
}

public function testEncodingAccessors()
fredden marked this conversation as resolved.
Show resolved Hide resolved
{
$header = new GenericHeader('Foo');
$this->assertEquals('ASCII', $header->getEncoding());
$header->setEncoding('UTF-8');
$this->assertEquals('UTF-8', $header->getEncoding());
}
}
1 change: 1 addition & 0 deletions test/Header/HeaderNameTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public function getFilterNames()
*/
public function testFilterName($name, $expected)
{
HeaderName::assertValid($expected);
$this->assertEquals($expected, HeaderName::filter($name));
}

Expand Down
49 changes: 49 additions & 0 deletions test/Header/IdentificationFieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ public function reversibleStringHeadersProvider()
];
}

public function invalidIds()
{
return [
[References::class, ["1234@local.machine.example\r\n"]],
[References::class, ["1234@local.machine.example", "3456@example.net\r\n"]],
[InReplyTo::class, ["3456@example.net\r\n"]],
];
}

/**
* @dataProvider stringHeadersProvider
* @param string $className
Expand All @@ -68,4 +77,44 @@ public function testSerializationToString($className, $headerString, $ids)
$header->setIds($ids);
$this->assertEquals($headerString, $header->toString());
}

/**
* @dataProvider stringHeadersProvider
* @param string $className
* @param string $headerString
* @param string[] $ids
*/
public function testEncodingAccessors($className, $headerString, $ids)
fredden marked this conversation as resolved.
Show resolved Hide resolved
{
/** @var IdentificationField $header */
$header = $className::fromString($headerString);
$this->assertEquals('ASCII', $header->getEncoding());
$header->setEncoding('UTF-8');
$this->assertEquals('ASCII', $header->getEncoding());
}

/**
* @dataProvider invalidIds
* @param string $className
* @param string[] $ids
*/
public function testSetIdsThrowsOnInvalidInput($className, $ids)
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
fredden marked this conversation as resolved.
Show resolved Hide resolved
/** @var IdentificationField $header */
$header = new $className();
$header->setIds($ids);
}

/**
* @dataProvider invalidIds
* @param string $className
* @param string[] $ids
*/
public function testFromStringRaisesExceptionOnInvalidHeader($className, $ids)
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
/** @var IdentificationField $header */
$header = $className::fromString('Foo: bar');
}
}
25 changes: 25 additions & 0 deletions test/Header/MessageIdTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ public function testSettingManually()

$expected = sprintf('<%s>', $id);
$this->assertEquals($expected, $messageid->getFieldValue());
$this->assertEquals($expected, $messageid->getId());
$this->assertEquals("Message-ID: $expected", $messageid->toString());
}

public function testAutoGeneration()
Expand All @@ -35,6 +37,15 @@ public function testAutoGeneration()
$this->assertContains('@', $messageid->getFieldValue());
}

public function testAutoGenerationWithServerVars()
{
$_SERVER['REMOTE_ADDR'] = '172.16.0.1';
$_SERVER['SERVER_NAME'] = 'server-name.test';
fredden marked this conversation as resolved.
Show resolved Hide resolved
$messageid = new Header\MessageId();
$messageid->setId();

$this->assertContains('@', $messageid->getFieldValue());
fredden marked this conversation as resolved.
Show resolved Hide resolved
}

public function headerLines()
{
Expand Down Expand Up @@ -77,4 +88,18 @@ public function testInvalidIdentifierRaisesException($id)
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
$header->setId($id);
}

public function testFromStringRaisesExceptionOnInvalidHeader()
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
Header\MessageId::fromString('Foo: bar');
}

public function testEncodingAccessors()
{
$header = new Header\MessageId();
$this->assertEquals('ASCII', $header->getEncoding());
$header->setEncoding('UTF-8');
$this->assertEquals('ASCII', $header->getEncoding());
}
}
15 changes: 15 additions & 0 deletions test/Header/MimeVersionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public function testDefaultVersion()
{
$mime = new Header\MimeVersion();
$this->assertEquals('1.0', $mime->getVersion());
$this->assertEquals('MIME-Version: 1.0', $mime->toString());
}

public function headerLines()
Expand Down Expand Up @@ -71,4 +72,18 @@ public function testRaisesExceptionOnInvalidVersionFromSetVersion($value)
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
$header->setVersion($value);
}

public function testFromStringRaisesExceptionOnInvalidHeader()
{
$this->expectException('Laminas\Mail\Header\Exception\InvalidArgumentException');
Header\MimeVersion::fromString('Foo: bar');
}

public function testEncodingAccessors()
{
$header = new Header\MimeVersion('1.0');
$this->assertEquals('ASCII', $header->getEncoding());
$header->setEncoding('UTF-8');
$this->assertEquals('ASCII', $header->getEncoding());
}
}
Loading