Hi Paul,
We encountered PHP compatibility warnings and a fatal search error
while
running SquirrelMail 1.4.23-svn on PHP 8.5.12. Reviewing the affected
paths
also
identified a broken literal-search charset retry and an error-rendering
call
that omits a required argument. I have attached a combined diff, six
smaller
patches, and isolated regression fixtures for review.
The production failures, source-derived findings, and validation limits
are
distinguished below. These are proposed compatibility/robustness fixes;
I am
not reporting a demonstrated security vulnerability.
Environment and patch baseline
------------------------------
* Production: Fedora 44, PHP CLI and PHP-FPM 8.5.12, Dovecot IMAP
backend.
* SquirrelMail: manually installed 1.4.23-svn snapshot originally dated
2024-07-30. The affected functions were compared with SM-1_4-STABLE.
* The public diff is rebased onto stable-branch files retrieved for
this
report. UPSTREAM_BASELINE.json records the source URLs and exact
before/
after SHA-256 values. I have not inferred an exact SVN revision
number.
* Deployment markers and diagnostic prefixes were removed from the
public
diff. Its functional changes match our deployed fixes. It contains no
host configuration, credentials, mailbox contents, or unrelated
changes.
* File/line references in the log examples below refer to our original
snapshot; current stable-branch line numbers can differ.
1. PHP 8.5 deprecations can produce secondary header/session warnings
-------------------------------------------------------------------
Files: functions/mime.php, functions/strings.php,
functions/imap_general.php
Patches: 0001 and 0002
Evidence: observed in production PHP logs.
The reported expressions are:
sq_deent(): chr($numval)
sq_mt_randomize(): (double) microtime()
map_yp_alias(): the backtick ypmatch expression
Representative sanitized messages:
PHP Deprecated: chr(): Providing a value not in-between 0 and 255 is
deprecated ... in functions/mime.php on line 1616
PHP Deprecated: Non-canonical cast (double) is deprecated, use the
(float) cast instead in functions/strings.php on line 532
PHP Deprecated: The backtick (`) operator is deprecated, use
shell_exec()
instead in functions/imap_general.php on line 1385
In our deployment these messages were followed by "headers already
sent"
warnings in page_header.php/i18n.php and a warning that session ini
settings
could no longer be changed. The NIS example does not have to be used by
an
account for its syntax to be deprecated when the file is compiled.
Minimal PHP 8.5 checks:
php -d error_reporting=-1 -d display_errors=1 -r 'chr(256);'
php -d error_reporting=-1 -d display_errors=1 -r 'echo (double)
microtime();'
php -d error_reporting=-1 -d display_errors=1 -l
functions/imap_general.php
Proposed changes:
chr(((int) $numval) & 255)
(float) microtime()
shell_exec("ypmatch $safe_username aliases")
The chr change deliberately preserves the existing low-byte behavior.
It is
not a change to Unicode/entity-decoding semantics. The shell command
and its
existing escapeshellarg call are preserved; the test does not execute
NIS.
Expected: normal message rendering/file loading without these
deprecations
or their resulting output/header cascade. Error reporting remains
enabled.
2. sqimap_search() can reach count() with an undefined message list
----------------------------------------------------------------
File: functions/imap_search.php, sqimap_search()
Patch: 0004
Evidence: production fatal error; isolated response fixtures.
Sanitized production messages:
PHP Warning: Undefined variable $messagelist in
functions/imap_search.php on line 148
PHP Fatal error: Uncaught TypeError: count(): Argument #1 ($value)
must
be of type Countable|array, null given in
functions/imap_search.php:148
The function unsets $messagelist and only assigns it when a response
begins
with "* SEARCH ". A failed command or missing/unrecognized SEARCH
response
can leave it undefined before count($messagelist). A response with a
trailing
space but no IDs can also yield an empty ID rather than a genuine empty
set.
This does not mean every ordinary no-match search fails: the existing
code
already handles some empty SEARCH forms.
Fixture reproduction: stub sqimap_run_command() to return an OK status
and
an empty response array, then call sqimap_search() with a simple FROM
search.
Also exercise NO/BAD/false responses, "* SEARCH\r\n", "* SEARCH \r\n",
multiple spaces, ordinary IDs, and unsolicited response lines.
Expected: a valid empty SEARCH response returns an empty array. Failed,
missing, or malformed responses display an error and return safely,
rather
than being silently presented as a successful empty search or throwing
a
PHP TypeError. Valid message IDs still reach fillMessageArray().
The patch initializes arrays, validates command status and response
shape,
recognizes empty SEARCH responses, validates returned IDs, and keeps
failures
visible through the existing UI helper and a diagnostic log entry. The
status-to-lowercase check also explicitly accepts a null status as a
string
before the failure path handles it.
3. Literal-search charset fallback prepares one query but sends another
---------------------------------------------------------------------
File: functions/imap_search.php, sqimap_search()
Patch: 0004
Evidence: source-derived finding, verified with an isolated command
stub;
not claimed as a separately captured production IMAP exchange.
If a literal search returns NO, the function modifies:
$original_search_literal['commands'][0]
to request US-ASCII, but its retry calls sqimap_run_literal_command()
with
$search_literal, repeating the original failing query.
Fixture reproduction: use a search string that requires the literal
path
and an enabled charset search. Make the first command return NO. Check
the
second command's first element. It should begin with:
SEARCH CHARSET "US-ASCII" ALL
The proposed one-argument change passes $original_search_literal on
retry.
The fixture then returns a valid ID and confirms it reaches the caller.
4. MIME body parsing passes null from array_shift() to trim()
-----------------------------------------------------------
File: functions/mime.php, mime_fetch_body()
Patch: 0005
Evidence: observed production warning; isolated FETCH-response
fixtures.
Observed:
PHP Deprecated: trim(): Passing null to parameter #1 ($string) of
type
string is deprecated in functions/mime.php on line 116
The parser uses trim(array_shift($data)). array_shift() returns null if
its
input has been exhausted. This may happen immediately or after
consuming
untagged lines without finding the expected FETCH data. A false
non-array
response also needs handling before array operations.
Proposed changes: show/log a retrieval failure for an initially missing
or
non-array response, and explicitly cast shifted lines to string before
trim.
The cast preserves the former null-to-empty-string behavior. This does
not
attempt to repair the underlying server/transport reason for missing
data.
Expected: no compatibility warning or array-operation failure, and a
visible
retrieval error when no response was received. Ordinary literal bodies
and
BODY[...] NIL keep their existing behavior.
5. read_body.php indexes cache state without valid request/mailbox
context
------------------------------------------------------------------------
File: src/read_body.php, before UIDVALIDITY/message-cache access
Patch: 0006
Evidence: production null-offset/undefined-variable warnings; guard
fixtures.
Observed messages include an array-offset access on null at line 792,
null array-offset deprecations at subsequent cache accesses, and an
undefined
$passed_id at lines 797/798.
The code dereferences $mbx_response['UIDVALIDITY'] and $passed_id
without
first
checking them. sqimap_mailbox_select() can return null for an empty
mailbox;
$passed_id is only assigned if the request supplies it.
Controlled web reproduction to check with an authenticated test
account:
request src/read_body.php without a usable message ID/mailbox, or
arrange
for selection to return no UIDVALIDITY. Do not use a real customer's
session
or change/delete message data for this test. Our included fixture
evaluates
the added guard only; it does not execute the whole page or establish a
plugin-free browser reproduction of the original event.
Expected: reject the invalid context with a useful error and stop
before
indexing the cache or fetching a message. Valid requests continue
unchanged.
The patch displays an error, closes the IMAP connection, and exits that
request.
6. sqimap_error_box() omits a required argument to error_box()
-----------------------------------------------------------
Files: functions/imap_general.php; signature in
functions/display_messages.php
Patch: 0003
Evidence: source-derived finding and isolated execution of the real
functions;
not presented as a fatal error captured from a production user request.
The helper ends with:
error_box($string);
but the renderer is declared:
function error_box($string, $color)
Calling that path under PHP 8 throws ArgumentCountError. The helper
already
declares global $color, so the proposed fix supplies it and ensures the
renderer file is loaded.
The helper also unconditionally invokes set_up_language(), which can
emit
Content-Type headers. For a late IMAP error after page output has
started,
the patch retains the initialized language and skips that call when
headers_sent() is true. It still initializes language before output.
The fixture executes the actual helper and actual renderer with small
stubs
for presentation dependencies. It checks visible output, the
two-argument
contract, and behavior both before and after output has begun.
Validation performed
--------------------
* The deployed changes passed PHP lint and isolated regression fixtures
on
PHP 8.5.12 before installation. PHP-FPM configuration validation and
reload
succeeded on October 25, 2026 at 19:30:59 -0400.
* The byte-decoding fixture covers every value from 0 through 65,535 in
both
decimal and hexadecimal entity forms, wrapping, multiple entities,
and
ordinary text. No PHP warning/deprecation was emitted by the patched
tests.
* The attached public combined diff and split series were independently
applied to the recorded stable-branch baseline with no offsets or
fuzz.
* After removing site-specific marker comments and diagnostic prefixes,
the
public patch's seven regression groups passed locally on PHP 8.3.6.
The
unpatched baseline failed six groups; byte compatibility passed there
because PHP 8.3 still permits the old chr input range. PHP 8.5
deprecation
behavior is supported separately by the production logs/tests above.
* These are isolated function/guard tests with synthetic backend
responses.
They are not the complete SquirrelMail suite, a live IMAP integration
test,
or confirmation of every authenticated browser/plugin flow.
Review points
-------------
The new error messages are currently English; please adapt them to the
project's gettext conventions before merging. The proposed error-return
behavior and placement of the message-request guard should also be
reviewed
against callers and supported older PHP versions. No new PHP 8-only
syntax
was intentionally introduced in application code, but our executed
runtime
tests were PHP 8.3 and 8.5, not every supported historical version.
The error-rendering patch precedes the search/FETCH/request changes
because
those paths rely on that helper. The series can be reviewed as six
smaller
changes rather than one combined diff. These files are for upstream
review;
they should not be applied again over our already-patched production
files.
Public references
-----------------
https://svn.code.sf.net/p/squirrelmail/code/branches/SM-1_4-STABLE/squirrelmail/
https://www.php.net/manual/en/migration85.deprecated.php
https://www.php.net/manual/en/function.chr.php
Thank you for reviewing these fixes.
-----
squirrelmail-users mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: [email protected]
List archives: http://news.gmane.org/gmane.mail.squirrelmail.user
List info (subscribe/unsubscribe/change options):
https://lists.sourceforge.net/lists/listinfo/squirrelmail-users
