message queue per user tripled, and the ceiling that holds it
If the client falls behind by more than COMM_N messages the history is lost:
it is sent back to the initial page, splash included. That happens when it
reconnects after a burst, for instance while the garbage collector removes
dozens of disconnected users at once and every removal produces a room
update.
Eighteen messages are few, but they could not be raised on their own: every
queued update carries the complete list of the people present
(standup_content), which with a full room is about 3 KB, and eighteen of
those were already 61 KB against the 65536 of SHM_DIMS_U_MAX. Raising COMM_N
without raising the ceiling would have made shm_put_var fail. The segments
grow by SHM_DIMS_U_DLT at a time, so whoever does not fill the queue pays
nothing.
WARNING: changing COMM_N means the queue indexes (step % COMM_N) no longer
match those the users were saved with. The persisted state has to be wiped:
without that, the tables stay in "sitreser" and never form again. Verified in
the field, in both directions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
the comet stream was closed without the final chunk
Streaming responses declare Transfer-Encoding: chunked and the data goes out
framed by chunked_content(), but on close the zero length chunk that
terminates the response was never sent.
With apache it went unnoticed: the daemon owned the client socket and simply
closed it. nginx instead parses the response, and every stream that renews
itself (RD_ENDTIME_DELTA, 240 seconds) looked truncated to it: "upstream
prematurely closed connection while reading upstream", one error line per
connected player per cycle.
The final chunk is added in User::stream_close(), which is already the place
where the daemon writes its last bytes before closing, and only for the
transports that really are chunked: websockets are not, and keep sending
their own close frame.
Measured: with 150 players the renewal of the streams produced 150 errors;
after the fix, 30 players and 30 stream reopenings give zero errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
Once the CHATT_MAXLINES ceiling was reached, every incoming message emptied
the box and then filled it line by line, assigning innerHTML on each turn.
Each += on innerHTML forces the browser to reserialise the content, reparse
it and rebuild the subtree: 41 rebuilds of the DOM per message, with a cost
that grows with the square of the number of lines.
Now the shift happens on the array and the box is rewritten once.
It came out of the load test: with 150 players chatting (15 messages a second
broadcast to everybody) a real browser saturated twelve cores and died, while
the daemon serving that traffic sat at 6% of one core. The bottleneck was the
client, not the server.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
custom.js is optional, and the page no longer demands it
index.php always loaded it, but custom.js holds the customisations of the
single site and may not be distributed: whoever installs from scratch got a
404 on every page load, on both branches of the layout.
The tag is now emitted only if the file is really there. The hook it uses
(custom_bedge) was already guarded on the javascript side, so without the
file the room behaves exactly as it always has.
Same approach as cookie_law.* and brisk_donate.txt: what belongs to a single
installation must not break the others.
Checked in the container: with the file the page references it and downloads
it (200), without the file the page is served complete, does not name it and
nginx logs no 404. Both occurrences were tried, the one on the entry page and
the one in the room.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
"su root -c" asks for the root password, which on a freshly installed debian
does not even exist: the first user administers with sudo. The two commands
were also nested (su root calling su postgres) with three levels of quoting
to get through, and the variables ended up in the string unprotected.
Now it goes straight to "sudo -u postgres" and the arguments are quoted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
the listen queue of the daemon sockets was the default one
With the php default backlog a single socket of the pool queues 111
connections and then refuses: measured with a burst of 300 simultaneous
connects. The frontend gets EAGAIN on the connect and has to fall back on
another socket of the pool, or return an error.
It showed up during the load test: with 300 entries close together nginx
logged "connect() to unix:...brisk0.sock failed (11: Resource temporarily
unavailable)". With the backlog at 511 the same burst goes through entirely
and the error does not come back.
Until now the pool of ten sockets masked the problem, spreading the
connections over ten short queues instead of one long queue.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
index.php reads it from FTOK_PATH/brisk_donate.txt, but that file was not in
the repository: whoever installed from scratch had no button, and the daemon
wrote a file_get_contents warning on every page load.
The html fragment now lives in data/, next to the other data files of the
installation, and INSTALL.sh copies it into FTOK_PATH only if it is not there
already, so as not to overwrite the one of the installation.
The file stays optional: index.php checks that it exists before reading it,
so whoever does not want the button does not get a dirty log (the code
already handled the case, but only after the warning had been written).
Checked in the container with a full installation, no longer just -W: the
file lands in FTOK_PATH, the page shows the form, the log stays clean;
without the file the page still loads with no form and no warning; and a
second installation does not touch the file already there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
the configuration template asked for a document that did not exist
$G_tos_vers stayed at "1.2" while the documents in Obj/doc/ were renamed
twice, in 2014 and in 2015, up to 1.4: the last time the two values were
aligned was in 2013.
The effect on a fresh installation: file_get_contents does not find the file,
writes a warning in the log and returns FALSE, and the user is asked to accept
an empty document anyway.
Checked in the container with 1.2 and with 1.4, everything else being equal:
with 1.2 the dialog arrives with no text and the warning is in the log, with
1.4 the text of the document arrives and the log is clean.
The switchover dates ($G_tos_dthard and $G_tos_dtsoft, november 2013) are left
as they are: they are in the past, so on a fresh installation the current
document has to be accepted right away, which is the intended behaviour.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
The sed looked for the BRISK_AUTH_CONF define in Obj/auth.phh, where it has
never been: it lives in Obj/dbase_file.phh. sed did not find the file, wrote
the error in the middle of the other installation lines, and the installation
carried on, so -a was ignored without anybody noticing.
The define is now looked for where it actually is, and if it is not found the
installation says so instead of keeping quiet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
"Only variables should be assigned by reference": the return value of a
function is not a variable, and none of the three methods called declares
that it returns a reference. With objects the & has been useless since php 5,
and every other place that calls get_user() already assigns without it.
The Notice came out on every card played (briskin5's index_wr.php) and was
invisible until the daemon log was cleaned up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
The daemon no longer receives the client descriptor handed over by the
frontend: nginx opens an ordinary http connection to it on one of the unix
sockets of the pool. Neither the php ancillary extension, nor
mod_proxy_fdpass2 for apache, nor ngx_http_fdpass_module for nginx, nor kTLS
are needed any more: the debian nginx package is enough. In exchange nginx
stays in the middle for the whole life of the connection, which in a comet
application is long.
The historic mode is one INSTALL.sh -D FALSE away.
The sample configuration was added under system/nginx/, and WARNING.txt was
rewritten: it only documented the old apache setup.
Two log traces had to be fixed before the direct mode could be used, because
they fired once per request instead of occasionally:
- "User associated with ID: N not found" was printed for every socket with
no user attached, that is for the head of every request. The message now
comes out only if the socket is in none of the known lists.
- "PP_REM" traced the removal of a pending page, which now happens on every
request: it moves under debug > 1.
Checked in the debian 13 container with the packaged nginx 1.26.3 (no added
module): static pages, the .htaccess protections rewritten in the
configuration, the login of five certified users, comet streaming, POST, and
three complete five player games over https recorded on postgresql. The
daemon log for a whole hand went from thousands of lines to about forty.
Behaviour under load and the reuse of connections between nginx and the
daemon remain to be checked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
Three defects in one line: it tied INSTALL.sh to apache (nginx uses "root",
not "DocumentRoot"), it took the first match in any VirtualHost without
knowing which site was the right one, and it did not cope with quotes around
the path.
The value, though, can already be deduced from the parameters: web_path ends
with prefix_path, so removing the latter from the former leaves the root.
No server to ask, and it works with multi segment prefixes too
(/var/www/html/games/brisk with -P /games/brisk/ gives /var/www/html).
The -R option was added to force an explicit value, and the old grep over the
file named by -A was kept as a last resort; it now recognises both
DocumentRoot and root and strips quotes and semicolons. If no route produces
a value, INSTALL.sh stops with a clear message instead of going on with an
empty string (which ended up producing requires of
"/Etc/brisk_spu.conf.pho").
The derived value is now printed among the parameters, like the others.
Checked in the container by running INSTALL.sh WITHOUT -A: it derives
/home/brisk/web, writes it into $DOCUMENT_ROOT inside spush/*.ph* and
donometer.php, and installs the files from docroot/ there. The site answers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
error.php and doc_download.php: $DOCUMENT_ROOT was never set
Both scripts include Obj/brisk.phh, which on line 94 does
require_once("$DOCUMENT_ROOT/Etc/".BRISK_CONF);
but neither of them set $DOCUMENT_ROOT. The path collapsed to
"/Etc/brisk_spu.conf.pho", the require failed and the page answered 500.
This is not a consequence of the port: the git history shows that
doc_download.php never had that line in two commits, and error.php does not
mention it at all. INSTALL.sh substitutes $DOCUMENT_ROOT only in spush/*.ph*
and donometer.php (line 445), not in these two.
NOTE: in the working copy doc_download.php carried a local fix that was never
committed, with the path written by hand
($DOCUMENT_ROOT="/home/nastasi/web"). Since INSTALL.sh distributes from the
working copy and not from git, that is probably what runs in production: a
fix that existed on one disk only and would have disappeared at the first
clone onto a new machine. This commit replaces it with the portable form.
Used the scheme already present in usermgmt.php, mailmgr.php,
briskin5/statadm.php and the others, which derive the value from $_SERVER: it
works both with mod_php and with php-fpm and does not depend on a hardcoded
path. $G_base = "" was added to doc_download.php too, which brisk.phh:95 needs
and which was missing.
Found by handing the pages not given to the daemon over to php-fpm, which
under apache were served by mod_php: both answered 500. After the fix: 200.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
docroot/: the files that belong in the root of the site
cookie_law.css and cookie_law.js are referenced by index.php with a leading
slash ("/cookie_law.js", lines 1047, 1048, 1221, 1222), so the browser asks
the DocumentRoot for them and not the subdirectory of the application.
INSTALL.sh installs everything inside $web_path, that is in /brisk/: putting
them under web/ would still land them in the wrong place.
They only lived as loose, untracked files in the working copy, and on a new
machine they would simply have been missing. nginx reported them as 404 on
every page load.
The docroot/ directory was created, its name declaring its destination, and
INSTALL.sh was taught to copy its content into $document_root: the same value
it already writes into $DOCUMENT_ROOT, so no second source of truth is
introduced.
Checked by deleting the two files from the container and running INSTALL.sh
again: it puts them back by itself.
NOTE: it depends on the "grep DocumentRoot" over the apache configuration
file (line 444), which remains the coupling to apache already pointed out.
Dropping apache means replacing it, and at that point it serves both uses.
custom.js stays out: it is untracked too, but referenced without a leading
slash, so it lives inside /brisk/ and it is enough to add it under web/ for
INSTALL.sh to distribute it with no further change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
alternative mode: nginx speaks http directly with the daemon
Prototype of an architecture that removes the descriptor handover. Turned on
with SPU_HTTP_DIRECT in brisk-spush.phh; the default stays FALSE, that is the
historic behaviour, and the two modes live side by side.
Today the descriptor the daemon receives is the one of the nginx->apache
connection, already in clear because nginx stripped the TLS one hop earlier.
With this mode nginx opens an ordinary http connection on the unix socket and
the daemon uses it directly.
What it takes away:
- php-ancillary, the C extension
- mod_proxy_fdpass2, the apache module
- ngx_http_fdpass_module, the nginx module
- apache as an intermediate layer
- the kTLS requirement, which would be needed if nginx were to hand over the
browser descriptor (encrypted) instead of the cleartext one towards apache
That is three pieces of bespoke C, each of which needed a port in this very
migration, plus a kernel requirement. The daemon becomes an ordinary http
server behind a reverse proxy.
The code:
- spu_head_end() and spu_head_to_info() repackage the request read from the
network in the same format the control channel produced ("The-Request:" in
front of the request line), so that spu_process_info() does not know where
the data comes from
- the head of the request is NOT read by blocking: there is a single event
loop for every player. The connection is registered in the
PENDINGPAGE_WAITHEAD state among the watched sockets and completed
incrementally, reusing the machinery already in place for the bodies of
partial POSTs; if the body is still missing, it moves on to
pendpage_try_addwait seamlessly
- trim() on the header value: it was missing, and real http writes
"Name: value" with a space, which would have ended up inside the value,
breaking the cookies and the comparisons on Upgrade
Checked in the container with nginx 1.26.3 and php 8.4, without apache: the
page, an authenticated login, comet streaming, POST, and a complete game over
HTTPS with five players, the auction, 40 cards and the score saved on
postgresql. Secure websockets work (101 Switching Protocols and frames from
the daemon), confirmed from chrome too. Zero errors in the daemon.
Not verified yet: behaviour under load, and the reuse of connections between
nginx and the daemon (in the tests keepalive towards the upstream is off).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
headers_render: three duplicated headers in every response
Http header names are case insensitive, php array keys are not. The
transports set "Content-type" with a lowercase t (transports.phh:568 and
:618, index.php:1025 and :1199) while headers_render() checked for
"Content-Type": the check never saw it and added the default anyway.
Same dynamic for Expires and Cache-Control, which force_no_cache() sets and
headers_render added again without checking at all.
Every response therefore went out with:
Content-Type: text/html + Content-type: text/html; charset="utf-8"
Expires: -1 + Expires: Mon, 26 Jul 1997 05:00:00 GMT
Cache-Control: no-cache + Cache-Control: no-cache, must-revalidate
With apache this went unnoticed: the daemon wrote the bytes straight to the
client and the browser applied the last value. Behind a reverse proxy the
response is parsed instead, the first value wins and the second is dropped:
the charset was lost, and in transports.phh:568 an application/xml was
replaced by text/html.
Fixed at the root rather than in the five calling places: headers_render
builds a map of the keys normalised to lowercase and uses it for every
check, so the defect does not come back if somebody writes "Content-type"
again tomorrow.
Found by putting nginx in front of the daemon: it reported "upstream sent
duplicate header line" 123 times. After the fix: zero.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
short <? tags turned into <?php: the page was unusable in the browser
109 occurrences of "<? echo ... ?>" in web/index.php (48) and
web/briskin5/index.php (61). Short tags only work with short_open_tag = On,
which is Off by default in php and is Off on debian 13; on the production
machine (debian 8) it is evidently On.
This is not a cosmetic problem. Without interpretation the text of the tag
ends up literally in the html, and in a javascript context such as
var g_tables_n = <? echo TABLES_N; ?>;
it becomes a syntax error that prevents the compilation of the WHOLE <script>
block. As a consequence none of the variables declared in there is created,
"sess" included, and the room page is unusable: the browser console reports
"sess is not defined" and the buttons do nothing.
Every src/href with cache busting was broken too
("commons.js?v=<? echo BSK_BUSTING; ?>"), and now renders properly
("commons.js?v=997ebdc").
Converted to <?php instead of turning short_open_tag on: the directive is
discouraged and not guaranteed, while the explicit form works everywhere.
All 109 occurrences had the identical shape "<? echo", and none of them fell
inside a php string, so the substitution is mechanical. Checked that the
generated page no longer contains uninterpreted tags.
Found by the user opening the site with a real browser: it is the first
defect that came from the javascript client, which the curl tests could not
detect because they do not execute the page.
NOTE: this commit also carries two pre-existing changes from the working
copy, unrelated to the tag conversion: the inclusion of custom.js in
index.php (two lines) and $brisk_donate passed to $brisk_vertical_menu. They
were already there and were picked up by the "git add" of the whole file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
A concatenation written in javascript style, with "+" instead of the php ".".
On php 5 and 7 it evaluated to 0+0 with a warning and printed "0"; since
php 8 adding two non numeric strings is a TypeError, and with no catch
anywhere the brisk-spush daemon died on the spot.
The branch is trivial to reach: a request to index_wr.php with an
unrecognised session is enough, an expired cookie for instance. Found by
sending a getchallenge after a daemon restart.
All similar cases were looked for: this is the only one in php code, the
other "+" between strings are inside javascript embedded in the html, or in
shell scripts quoted in comments.
Found by playing a real game in the container: five authenticated users,
table 4, the auction, 40 cards played, score saved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
fixes that showed up by actually running the application on debian 13
Found by bringing the whole stack up in a container: apache 2.4.68 with
mod_proxy_fdpass2, the brisk-spush daemon on php 8.4 with the ancillary
extension, postgresql 17. None of these was visible with the lint, with
loading the include chain, or with the tests on the objects: they only show
up by starting the daemon and serving a real request.
sac-a-push.phh: fatal when the daemon starts
sig_handler() was registered with pcntl_signal() as
array("Sac_a_push", "sig_handler"), that is in static form, but declared
non static. Since php 8 that is no longer a valid callable and
pcntl_signal() raises a TypeError: the daemon died before opening a socket.
It is the same class of problem as the 15 static calls already fixed, but
with the array() syntax: the check I had written looked for "Class::method"
and did not see it. The other two callables in that form were checked as
well (IPClassItem::compare and Cookie::create): both already static.
INSTALL.sh: Etc/ was born exposed on the web
The Etc directory holds the configuration with $G_dbauth, that is the
database credentials in clear, and it falls inside the DocumentRoot. The
.pho extension is not associated with php, so the file was served as plain
text: checked, HTTP 200 with the content. In production it is protected only
because someone added a .htaccess by hand; a fresh installation was born
without one. INSTALL.sh now creates it, in the apache 2.4 form with a 2.2
fallback. After the change: HTTP 403.
WARNING.txt: the suggested ProxyPass lines did not work
It is the text INSTALL.sh prints to the administrator as the configuration
to write, and it was wrong in three ways:
- "fd:///path" is refused at configuration time by apache 2.4.68
("ProxyPass URL must be absolute!"); "fd://localhost/path" is needed
- it mentioned a single "brisk.sock", from before the pool existed: the
path is the prefix and the module appends "<N>.sock" to it
- the hardcoded path /var/www/brisk-priv ignored the -U option
Rewritten with the form verified to work, plus the note that the first
argument must be an exact path and not a prefix with a trailing slash.
The file also lists, and this part was already right, which urls go to the
daemon: index.php, index_wr.php, index_rd.php, index_rd_wss.php and the
matching ones under briskin5. Everything else is served by apache.
Final check in the container: GET /brisk/index.php answers 200 with the game
page (19725 bytes), the .css are served by apache, Obj/, spush/ and
briskin5/Obj/ answer 403, Etc/ answers 403, and the daemon does not emit a
single warning or deprecation while serving the requests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
Found by running the code against a real database: neither the lint nor
loading the sources could see them.
UPDATE ... SET (col) = (val)
Since postgresql 10 the parenthesised form on a SINGLE column is an error
("source for a multiple-column UPDATE item must be a sub-SELECT or ROW()
expression"): (val) is not a ROW but a parenthesised expression. The multi
column form is still valid, checked on the server: of the 11 parenthesised
UPDATEs in the project only 4 need fixing, the other 7 are left alone.
dbase_pgsql.phh SET (lintm) user_update_login_time()
SET (pass) user_update_passwd()
SET (tos_vers) user_tos_update()
SET (game_cnt) bin5_points_save()
sql.d/085-tourn-update.sql two SET (name)
This is not a consequence of the php 8 port: they were already broken on any
postgresql >= 10. They cover password recovery and the acceptance of the
terms of service.
int2four()
The literal 0xffffffff00000000 is above PHP_INT_MAX, so php treats it as a
float and the or converts it back to int: since 8.1 that is the "Implicit
conversion from float to int loses precision" deprecation, emitted on every
call (the function sits in the self-registration check path). Rewritten with
~0xffffffff, same bit pattern but an integer. Identical values, compared on
0, 1, 0x7fffffff, 0x80000000, 0xc0a80001 and 0xffffffff.
Checked against a real database (postgresql 17, schema rebuilt from scratch
with sql/builder.sh: 18 files, 12 tables, 6 views, 0 errors): connection,
queries, user_add, login_exists, getrecord_bylogin, the three fixed UPDATEs,
the two multi column ones, transactions and selfreg. No warnings, no
deprecations. The error branch of BriskDB::query() was checked too, by
forcing a query on a non existing table: it logs with pg_last_error(), does
not raise a TypeError, and the connection survives the recovery.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
The code was written for php 5. Minimal changes to make it run cleanly on
8.4, with no restructuring.
Fatal errors
- split() -> explode() (removed in 7.0), 5 places
- "$x =& new Class()" -> "= new" (removed in 7.0), 7 places
- 41 php4 style constructors -> __construct(). A non obvious case: Bin5_user
defined "function User() {}", which on php5 was its constructor because it
overrode the slot inherited from User; that one was renamed too.
- 15 static calls to non static methods (Challenges::load_data(),
Hardbans::add(), Table::create(), ...): E_STRICT on php5, Error since 8.0.
"static" added to the 8 declarations, none of them uses $this.
- 5 overrides with incompatible signatures (spawn, copy, load_step,
unproxy_step, page_sync): E_STRICT on php5, fatal since 8.0. The useless
"&" on objects were dropped and the parameters of three methods reordered,
with the two call sites adjusted.
- dbase_pgsql.phh: pg_result_status($res) was called in the branch where
$res is FALSE. Since 8.0 results are \PgSql\Result objects and no longer
resources, so it is not a warning any more but a fatal TypeError - and in
the connection recovery path, of all places. Replaced with pg_last_error().
- dbase_pgsql.phh: "${rules_name}::game_description(...)" was a variable
variable whose name came from an undefined constant; on php5 it degraded to
a string with a notice and resolved to $rules_name by accident, on php8 it
is a fatal Error.
- dbase_file.phh: define() with an unquoted constant name, same mechanism.
- usermgmt.php: "break" outside any loop. On php5 it was a runtime fatal,
since 7.0 it is a compile time one: the file did not load any more.
Deprecations
- 245 "var $prop" -> public
- 29 occurrences of "${var}" inside strings -> "{$var}" (8.2)
- 29 dynamic properties declared (8.2). User declared $brisk but the code
always uses $room: renamed, nobody reads $user->brisk.
- 53 pg_numrows() -> pg_num_rows(): the alias is deprecated in 8.4
- strftime() -> date(), shmop_close() -> unset()
- room_join_wakeup(): removed a default followed by a mandatory parameter
mbstring.func_overload
It was set to 7 in the .htaccess files and was removed in 8.0. All 62 call
sites of strlen/substr/strpos were examined: they are either pure ASCII or
deliberately byte oriented, and moving to php8 fixes them, given that the
websocket frame parsing in transports.phh and the fwrite accounting in
sac-a-push.phh would have been wrong under overload. No change needed: where
character semantics were required the author already used explicit mb_*.
The only exception is index_wr.php, where mail() is no longer remapped onto
mb_send_mail(): mb_encode_mimeheader() was added on the subject and on the
user name, which otherwise ended up as raw UTF-8 in the headers.
Configuration (debian 13)
- .htaccess: func_overload removed, internal_encoding/http_input replaced by
default_charset; the php_value block now sits inside <IfModule mod_php.c>
because with PHP-FPM apache would answer 500
- the three .htaccess that protect the sources used the apache 2.2 syntax
(Order/Deny), which needs mod_access_compat: now "Require all denied" with
a fallback
- system/etc_php5_conf.d_mbstring.ini -> etc_php8.4_conf.d_brisk.ini
- INSTALL.sh: "php5 -l" -> "php -l"
Checked: php -l clean on 64 files; the whole include chain of the daemon
loads with E_ALL without warnings or deprecations; the tests in test/ pass.
Not yet verified against a database: dbase_pgsql.phh was only checked
statically.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
The class.phpmailer.php in web/Obj/ was the 5.1 release from 2009 and does
not run on php 8: it uses each() (removed in 8.0),
get_magic_quotes_runtime() and set_magic_quotes_runtime() (removed in 8.0)
and php4 style constructors.
Replaced by PHPMailer 7.1.1 in web/Obj/PHPMailer/ (src/ plus the italian
language file and the LICENSE). 7.x was chosen over 6.x because 7.0.0 is
identical to 6.11.1: the major bump only signals a compatibility break for
those who extend the class (lang(), setLanguage() and $language became
static), and here PHPMailer is not extended. It is the line maintained for
php 8.4. No composer: the project does not use it, and INSTALL.sh already
copies files recursively - only LICENSE and VENDOR.txt had to be added to
the list of copied names.
mail.phh adjusted: namespace PHPMailer\PHPMailer, explicit require of the
three files (no autoloader), setFrom() instead of assigning From/FromName
directly.
A missing catch was added too: brisk_mail() builds PHPMailer with
exceptions=TRUE, so send() throws instead of returning FALSE, but none of
the 7 callers catches and all of them test for "== FALSE". A delivery error
killed the spush daemon. The exception is now logged and reported as FALSE,
which is what the callers already expected.
NOTE: msgHTML() overwrites AltBody with its own conversion of the html, so
the text passed to brisk_mail() is discarded. This was already the case
with 5.1, and the behaviour is left untouched (see the comment in the file).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE
projects / brisk.git / log