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

Commit e1c049e

Browse files
MellieVTweierophinney
authored andcommitted
Fix HeaderValue throwing an exception on legal characters
- Changes the ASCII out-of-range checks to use `> 127` instead of `> 126`, as 127 is the last ASCII character. - Removed the `testCannotBeConstructed()` test case, as irrelevant. - Added a data provider to test that wrapping header lines using `\r\n\t` is valid. It failed, and, as such, I updated the `isValid()` code to check for both ASCII 32 (SP) and 9 (TAB) when checking for line wrapping.
1 parent 393b43c commit e1c049e

File tree

5 files changed

+24
-20
lines changed

5 files changed

+24
-20
lines changed

.php_cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22
$finder = Symfony\CS\Finder\DefaultFinder::create()
3+
->in('src')
4+
->in('test')
35
->notPath('TestAsset')
46
->notPath('_files')
57
->filter(function (SplFileInfo $file) {

src/Header/HeaderValue.php

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,18 @@ private function __construct()
2828
public static function filter($value)
2929
{
3030
$result = '';
31-
$tot = strlen($value);
31+
$total = strlen($value);
3232

3333
// Filter for CR and LF characters, leaving CRLF + WSP sequences for
3434
// Long Header Fields (section 2.2.3 of RFC 2822)
35-
for ($i = 0; $i < $tot; $i += 1) {
35+
for ($i = 0; $i < $total; $i += 1) {
3636
$ord = ord($value[$i]);
37-
if (($ord < 32 || $ord > 126)
38-
&& $ord !== 13
39-
) {
37+
if ($ord === 10 || $ord > 127) {
4038
continue;
4139
}
4240

4341
if ($ord === 13) {
44-
if ($i + 2 >= $tot) {
42+
if ($i + 2 >= $total) {
4543
continue;
4644
}
4745

@@ -72,27 +70,28 @@ public static function filter($value)
7270
*/
7371
public static function isValid($value)
7472
{
75-
$tot = strlen($value);
76-
for ($i = 0; $i < $tot; $i += 1) {
73+
$total = strlen($value);
74+
for ($i = 0; $i < $total; $i += 1) {
7775
$ord = ord($value[$i]);
78-
if (($ord < 32 || $ord > 126)
79-
&& $ord !== 13
80-
) {
76+
77+
// bare LF means we aren't valid
78+
if ($ord === 10 || $ord > 127) {
8179
return false;
8280
}
8381

8482
if ($ord === 13) {
85-
if ($i + 2 >= $tot) {
83+
if ($i + 2 >= $total) {
8684
return false;
8785
}
8886

8987
$lf = ord($value[$i + 1]);
9088
$sp = ord($value[$i + 2]);
9189

92-
if ($lf !== 10 || $sp !== 32) {
90+
if ($lf !== 10 || ! in_array($sp, [9, 32], true)) {
9391
return false;
9492
}
9593

94+
// skip over the LF following this
9695
$i += 2;
9796
}
9897
}

test/Header/ContentTypeTest.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@
1010
namespace ZendTest\Mail\Header;
1111

1212
use Zend\Mail\Header\ContentType;
13-
use Zend\Mail\Header\Exception\InvalidArgumentException;
14-
use Zend\Mail\Header\HeaderInterface;
15-
use Zend\Mail\Header\UnstructuredInterface;
1613

1714
/**
1815
* @group Zend_Mail

test/Header/HeaderValueTest.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ public function getFilterValues()
3030
array("This is a\r\r test", "This is a test"),
3131
array("This is a \r\r\n test", "This is a \r\n test"),
3232
array("This is a \r\n\r\ntest", "This is a test"),
33-
array("This is a \r\n\n\r\n test", "This is a \r\n test")
33+
array("This is a \r\n\n\r\n test", "This is a \r\n test"),
34+
array("This is a test\r\n", "This is a test"),
3435
);
3536
}
3637

@@ -50,13 +51,18 @@ public function validateValues()
5051
array("This is a\r test", 'assertFalse'),
5152
array("This is a\n\r test", 'assertFalse'),
5253
array("This is a\r\n test", 'assertTrue'),
54+
array("This is a\r\n\ttest", 'assertTrue'),
5355
array("This is a \r\ntest", 'assertFalse'),
5456
array("This is a \r\n\n test", 'assertFalse'),
5557
array("This is a\n\n test", 'assertFalse'),
5658
array("This is a\r\r test", 'assertFalse'),
5759
array("This is a \r\r\n test", 'assertFalse'),
5860
array("This is a \r\n\r\ntest", 'assertFalse'),
59-
array("This is a \r\n\n\r\n test", 'assertFalse')
61+
array("This is a \r\n\n\r\n test", 'assertFalse'),
62+
array("This\tis\ta test", 'assertTrue'),
63+
array("This is\ta \r\n test", 'assertTrue'),
64+
array("This\tis\ta\ntest", 'assertFalse'),
65+
array("This is a \r\t\n \r\n test", 'assertFalse'),
6066
);
6167
}
6268

@@ -81,7 +87,7 @@ public function assertValues()
8187
array("This is a\r\r test"),
8288
array("This is a \r\r\n test"),
8389
array("This is a \r\n\r\ntest"),
84-
array("This is a \r\n\n\r\n test")
90+
array("This is a \r\n\n\r\n test"),
8591
);
8692
}
8793

test/MessageTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ public function testHeaderUnfoldingWorksAsExpectedForMultipartMessages()
763763
$text->encoding = Mime::ENCODING_QUOTEDPRINTABLE;
764764
$text->disposition = Mime::DISPOSITION_INLINE;
765765
$text->charset = 'UTF-8';
766-
766+
767767
$html = new MimePart('<b>Test content</b>');
768768
$html->type = Mime::TYPE_HTML;
769769
$html->encoding = Mime::ENCODING_QUOTEDPRINTABLE;

0 commit comments

Comments
 (0)