Skip to content

Commit 8480fed

Browse files
committed
Move string field defaults into EventInput
Review feedback: with EventInput in place, the remaining $_POST defaulting block belongs there too, leaving a single write to $_POST. Add STRING_FIELDS to EventInput and rename normalizeNumericFields() to normalize(), since it now covers both field groups. The string defaults use ??=, which matches the previous !isset() check -- an explicit null is treated as missing either way. Add tests for the string defaults, including that a falsy '0' is preserved, and retarget the untouched-fields test at an unmanaged field now that type/email are normalized.
1 parent 9ffddc4 commit 8480fed

3 files changed

Lines changed: 147 additions & 18 deletions

File tree

src/Events/EventInput.php

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace phpweb\Events;
6+
7+
final class EventInput
8+
{
9+
/**
10+
* The numeric event fields (start/end date parts and recurrence) that must be
11+
* integers before being passed to checkdate() or mktime().
12+
*/
13+
private const NUMERIC_FIELDS = [
14+
'sday', 'smonth', 'syear', 'eday',
15+
'emonth', 'eyear', 'recur', 'recur_day',
16+
];
17+
18+
/**
19+
* The free-text event fields, which default to an empty string so the form can
20+
* be rendered and validated without E_WARNING on undefined array keys.
21+
*/
22+
private const STRING_FIELDS = [
23+
'type', 'country', 'category', 'email', 'url', 'ldesc', 'sdesc',
24+
];
25+
26+
/**
27+
* Normalize a raw event submission so every known field is present and of the
28+
* expected type.
29+
*
30+
* Untrusted numeric input (non-numeric strings, arrays, missing values) would
31+
* otherwise reach checkdate()/mktime() and throw a TypeError under PHP 8.
32+
* Anything that is not a numeric value becomes 0, i.e. an invalid date that the
33+
* normal form validation already rejects. Missing text fields become an empty
34+
* string. Unknown fields are left untouched.
35+
*
36+
* @param array<string, mixed> $post
37+
* @return array<string, mixed>
38+
*/
39+
public static function normalize(array $post): array
40+
{
41+
foreach (self::NUMERIC_FIELDS as $field) {
42+
$value = $post[$field] ?? null;
43+
$post[$field] = is_numeric($value) ? (int) $value : 0;
44+
}
45+
46+
foreach (self::STRING_FIELDS as $field) {
47+
$post[$field] ??= '';
48+
}
49+
50+
return $post;
51+
}
52+
}

submit-event.php

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
<?php
2+
use phpweb\Events\EventInput;
3+
24
$_SERVER['BASE_PAGE'] = 'submit-event.php';
35
include_once __DIR__ . '/include/prepend.inc';
46
include_once __DIR__ . '/include/posttohost.inc';
@@ -9,24 +11,10 @@
911
$errors = [];
1012
$process = [] !== $_POST;
1113

12-
// Avoid E_NOTICE errors on incoming vars if not set
13-
$vars = [
14-
'sday', 'smonth', 'syear', 'eday',
15-
'emonth', 'eyear', 'recur', 'recur_day',
16-
];
17-
foreach ($vars as $varname) {
18-
if (empty($_POST[$varname])) {
19-
$_POST[$varname] = 0;
20-
}
21-
}
22-
$vars = [
23-
'type', 'country', 'category', 'email', 'url', 'ldesc', 'sdesc',
24-
];
25-
foreach ($vars as $varname) {
26-
if (!isset($_POST[$varname])) {
27-
$_POST[$varname] = "";
28-
}
29-
}
14+
// Default the missing fields and coerce the numeric date/recurrence fields to
15+
// integers, so untrusted input can be passed safely to checkdate()/mktime() below
16+
// instead of throwing a TypeError.
17+
$_POST = EventInput::normalize($_POST);
3018

3119
// We need to process some form data
3220
if ($process) {
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace phpweb\Test\Unit\Events;
6+
7+
use phpweb\Events\EventInput;
8+
use PHPUnit\Framework;
9+
10+
#[Framework\Attributes\CoversClass(EventInput::class)]
11+
final class EventInputTest extends Framework\TestCase
12+
{
13+
public function testCoercesNumericStringsToIntegers(): void
14+
{
15+
$result = EventInput::normalize(['smonth' => '12', 'sday' => '25', 'syear' => '2026']);
16+
17+
self::assertSame(12, $result['smonth']);
18+
self::assertSame(25, $result['sday']);
19+
self::assertSame(2026, $result['syear']);
20+
}
21+
22+
public function testCoercesNonNumericStringsToZero(): void
23+
{
24+
$result = EventInput::normalize(['smonth' => 'January', 'sday' => 'abc', 'syear' => '12abc']);
25+
26+
self::assertSame(0, $result['smonth']);
27+
self::assertSame(0, $result['sday']);
28+
self::assertSame(0, $result['syear']);
29+
}
30+
31+
public function testCoercesArrayValuesToZero(): void
32+
{
33+
$result = EventInput::normalize(['smonth' => ['x'], 'sday' => [], 'syear' => ['1', '2']]);
34+
35+
self::assertSame(0, $result['smonth']);
36+
self::assertSame(0, $result['sday']);
37+
self::assertSame(0, $result['syear']);
38+
}
39+
40+
public function testDefaultsMissingNumericFieldsToZero(): void
41+
{
42+
$result = EventInput::normalize([]);
43+
44+
foreach (['sday', 'smonth', 'syear', 'eday', 'emonth', 'eyear', 'recur', 'recur_day'] as $field) {
45+
self::assertSame(0, $result[$field], $field);
46+
}
47+
}
48+
49+
public function testDefaultsMissingStringFieldsToEmptyString(): void
50+
{
51+
$result = EventInput::normalize([]);
52+
53+
foreach (['type', 'country', 'category', 'email', 'url', 'ldesc', 'sdesc'] as $field) {
54+
self::assertSame('', $result[$field], $field);
55+
}
56+
}
57+
58+
public function testKeepsSuppliedStringFields(): void
59+
{
60+
$result = EventInput::normalize(['email' => 'a@b.com', 'type' => 'multi', 'sdesc' => '0']);
61+
62+
self::assertSame('a@b.com', $result['email']);
63+
self::assertSame('multi', $result['type']);
64+
self::assertSame('0', $result['sdesc']);
65+
}
66+
67+
public function testLeavesUnknownFieldsUntouched(): void
68+
{
69+
$result = EventInput::normalize(['sname' => 'PHP UK', 'smonth' => '5']);
70+
71+
self::assertSame('PHP UK', $result['sname']);
72+
self::assertSame(5, $result['smonth']);
73+
}
74+
75+
/**
76+
* Regression test for the production fatal:
77+
* Uncaught TypeError: checkdate(): Argument #1 ($month) must be of type int, string given
78+
*
79+
* After normalization the date parts are always integers, so passing them to
80+
* checkdate() (and mktime()) can no longer throw a TypeError; garbage input
81+
* simply becomes an invalid (0) date that the normal validation rejects.
82+
*/
83+
public function testNormalizedValuesAreSafeForCheckdate(): void
84+
{
85+
$result = EventInput::normalize(['smonth' => 'notamonth', 'sday' => '1', 'syear' => '2026']);
86+
87+
self::assertFalse(checkdate($result['smonth'], $result['sday'], $result['syear']));
88+
}
89+
}

0 commit comments

Comments
 (0)