Released version 3.5-dev2 with the following main changes :
- MINOR: proxy: permit to report version info for option deprecation
- MAJOR: proxy: remove support for "dispatch" and "transparent" proxy keywords
- MEDIUM: cli/show-fd: no longer accept filtering for dispatch mode
- CLEANUP: connection: remove some checks for objt_proxy(conn->target)
- CLEANUP: backend: drop checks for OBJ_TYPE_PROXY in connect() code
- CLEANUP: trace: remove backend retrieval attempt from conn->target
- MAJOR: ot: remove deprecated OpenTracing support
- BUG/MEDIUM: h3: fix trace crash on frontend response headers
- CI: github: remove OpenTracing leftovers
- BUG/MEDIUM: mux_quic: fix memory leak of rx app_buf on stream free
- MINOR: ssl: export ssl_sock_init_srv()
- MEDIUM: httpclient: initialize the httpclient with default SSL values
- BUG/MINOR: hq-interop: fix transcoding of wrapping response buffer
- BUG/MINOR: hq-interop: support transcoding of absolute URI
- BUG/MINOR: h3: adjust HTTP headers traces
- MINOR: mux_quic: add minimal traces for QUIC MUX init/release
- MINOR: hq-interop: add request start-line traces
- MINOR: hq-interop: trace transcoding of response status line
- MINOR: hq-interop: trace HTX headers
- BUG/MEDIUM: server: initialise agent.health in srv_settings_init()
- BUG/MINOR: sample: set SMP_F_CONST on srv_name fetch
- MINOR: server: distinguish name references with new SRV_F_NAME_REFD flag
- MEDIUM: server: add 'set server name' CLI command for runtime server renaming
- REGTESTS: server: add test for 'set server name' CLI command
- DOC: server: document 'set server name' CLI command
- BUG/MEDIUM: servers: Use a refcount for port_range and free it properly
- MINOR: hbuf: new lightweight hbuf API
- MINOR: init: add no listener mode
- MINOR: trace: add definitions for haload streams
- MINOR: hldstream: add definition of hldstream struct objects
- MINOR: obj_type: add OBJ_TYPE_HALOAD for haload stream objects
- MINOR: stconn: add sc_hastream() and __sc_hastream() helpers
- MINOR: stconn: export sc_new()
- MINOR: server: export functions used during server initialization
- MINOR: haload: import source code and documentation
- MINOR: log: add app_log_raw() and send_log_raw() for binary-safe logging
- BUG/MINOR: init: fix default global settings being overwritten by -G
- BUG/MINOR: tools: fix invalid character detection in strl2ic()
- BUG/MAJOR: htx: Don't swap buffers for empty HTX message with an error
- CLEANUP: applet/http-client: Don't needlessly copy HTX flags after htx_xfer()
- BUG/MINOR: mux-quic: Fix handling EOM after in qcs_http_rcv_buf()
- BUG/MINOR: http-htx: Don't by-pass HTX API when merging cookie values
- BUG/MEDIUM: h3: fix parser desync on error with multiple frames
- BUG/MINOR: mux_quic: prevent multiple STOP_SENDING emission per stream
- BUILD: quic: workaround a gcc bug saying "maybe used uninitialized" when USE_TRACE=0
- CLEANUP: traces: get rid of a few rare empty args in TRACE calls
- MINOR: compiler: add a macro to ignore all arguments
- MINOR: trace: always pretend to use args when disabled
- BUILD: ssl: avoid a wrong null deref warning in ssl_sock_handshake
- CLEANUP: haproxy: remove -dt parsing and help when !USE_TRACE
- CLEANUP: mux-h2/traces: remove unused trace code when building without USE_TRACE
- CLEANUP: debug/trace: remote "debug dev trace" when USE_TRACE is not set
- BUG/MINOR: trace/quic_frame: use buf, not trace_buf in chunk_frm_appendf()
- CLEANUP: trace/h3: allow to disable traces in H3
- CLEANUP: trace/config: do not register section "traces" with USE_TRACE=0
- CLEANUP: trace/tree-wide: drop trace decoding/definition when USE_TRACE=0
- BUILD: makefile: only build trace.c and ssl_trace.c when USE_TRACE is set
- BUILD: makefile: add macros enable_opts and disable_opts
- BUILD: makefile: add an option to enable or disable HTTP/2 (USE_H2)
- BUILD: makefile: add an option to enable or disable FCGI (USE_FCGI)
- BUILD: makefile: add an option to enable or disable SPOE (USE_SPOE)
- BUILD: makefile: add a new generic target "tiny"
- BUG/MEDIUM: mux_quic: do not free QCS if STOP_SENDING to sent
- MINOR: mux_quic: use separate error code for STOP_SENDING
- MINOR: mux_quic: adjust shut stream callback
- BUG/MEDIUM: mux_quic: complete stream shutdown for read channel
- BUG/MINOR: quic: ignore STREAM after MUX closure on BE side
- BUG/MEDIUM: fd: Fix a deadlock when closing other tgroups fds
- MEDIUM: quic: remove deprecated keywords
- DEV: patchbot: keep the review start in sync with the radios on reload
- DEV: patchbot: only display the first 8 chars of the commit id
- DEV: patchbot: pass the branch version to the generated page
- DEV: patchbot: let the page fetch the shared review state
- DEV: patchbot: let the page save the review edits to the server
- DEV: patchbot: let the page edit and delete whole notes
- DEV: patchbot: gray the save button when there is nothing to save
- DEV: patchbot: don't pretend a save succeeded when the server ignored it
- DEV: patchbot: tolerate polluted save responses and show server warnings
- DEV: patchbot: repeat the syncing buttons at the bottom of the page
- DEV: patchbot: update: add an awk backend to persist review edits
- DEV: patchbot: update: return the stored overlay as JSON on GET
- DEV: patchbot: update: support replacing a whole note blob (setnotes)
- DEV: patchbot: update: report git commit failures in the save response
- DEV: patchbot: update: report the exact git error to the user
- DEV: patchbot: update: never write to stderr, thttpd sends it first
- DEV: patchbot: update: name the touched commits in the storage messages
- DEV: patchbot: document the shared review persistence
- BUG/MINOR: haload: fix spurious task wakeup in hld_strm_task()
- BUG/MINOR: hbuf: treat unexpected escape sequences as literals
- BUG/MEDIUM: tcpcheck: Add proxy used for healthcheck sections in proxies list
- BUG/MINOR: sample: Fix a possible underflow on be2hex for large chunk size
- MINOR: chunks: Add function to get a large/regular chunk depending on a buffer
- BUG/MEDIUM: chunk: Review chunks usage to not retrieve a large buffer by error
- MINOR: htx: Add a field to save the headers data size
- MEDIUM: htx: Be sure size of headers never exceed regular buffer on update
- BUG/MINOR: stream: Fix custom timeouts initialization when setting backend
- REGTESTS: Improve script testing the set-timeout action
- BUG/MINOR: stream: Fix custom max-retries initialization when setting backend
- MINOR: stream: Add be_max_retries/cur_max_retries sample fetch functions
- BUG/MINOR: http-conv: Make url-dec failed if no space for trailing null byte
- MAJOR: mworker: remove deprecated "master-worker" global keyword
- DOC: haterm: add a missing 'haterm' build target on an example
- DOC: readme: add a pointer to haterm/haload docs
- MINOR: ocsp: Do not see ocsp loading failures as fatal anymore
- REGTESTS: Remove unused `add_range_to_test_list` function from `scripts/run-regtests.sh`
- REGTESTS: Remove unused `_version` function from `scripts/run-regtests.sh`
- REGTESTS: Migrate REQUIRE_OPTION to `haproxy -cc`
- REGTESTS: Remove support for `REQUIRE_OPTION` from scripts/run-regtests.sh
- REGTESTS: Migrate `REQUIRE_SERVICE=prometheus-exporter` to a `feature(PROMEX)` check
- REGTESTS: Remove support for `REQUIRE_SERVICE` from scripts/run-regtests.sh
- CI: github: update vmactions/freebsd-vm to 14.4
- MINOR: haload: move statistics header printing to mtask_cb
- MINOR: haterm: add note about QUIC usage on SSL port
- MINOR: haload: allow "0" shortcut for IPv4 bind address
The address parser was failing when using "0" as a shortcut for INADDR_ANY.
This patch intercepts the "0" prefix and normalizes it to "0.0.0.0"
before the address is parsed.
Move the header printing logic from hld_init to mtask_cb using a static flag.
This ensures the header is displayed only when the main task starts
processing, avoiding issues where the header might be printed before other
haproxy initialization messages or unexpected early exits.
Until now, if the call to 'ssl_sock_load_ocsp' raised an error, because
of a missing issuer certificate for instance, we would send a fatal
error code and the init would fail.
With this patch we only consider this as a non-critical error and we
will send a warning instead. In such a case the OCSP response will not
be loaded and stapling or auto update features will not work for the
certificate.
People landing on the page seeking for help on how to build haterm/haload
need to have a direct entry on this page, so let's add a line pointing to
both docs.
This keyword, when alone, was already deprecated in 3.3 and marked
for removal in 3.5. Now using it results in an error message inviting
to start haproxy with -W or -Ws instead.
In addition, "master-worker exit-on-failure", which used to automatically
enable the master-worker mode, now doesn't set it and complains if used
without. -W/-Ws. A special case was made for config checks though (-c).
Also worth noting that "daemon" applies to master as well as max-reloads,
and will soon need to be adjusted.
for url-dec converter, a trailing null byte is added at the end of the input
sample because it is requested by url_decode() function. However, when the
buffer was full, the last byte was crushed by the trailing null byte. In
that case, the last character was lost and not decoded.
Now, the converter just fails by returning 0.
This patch could be backported to all supported versions but it is a very
minor issue.
These new samples can be used to get, resepectively, the backend value for
the maximum connection retries and the current value applied to the stream.
They could be used to know if a "set-retries" action was performed.
A dedicated reg-test was also added.
It is possible to overwrite the configured max-retries value with the
"set-retries" action. However there is an issue with the listeners. When the
backend is set, this value is reset with the backend value. However, when
the backend is unchanged, the custom value must be preserved.
To fix the issue, when the stream is created, we set the max-retries to the
listener value. It remains 0 for pure-frontend. And the backend value is
used only if it is not the same proxy.
The documentation of set-retries action was improved.
This patch should be backported as far as 3.2.
First, instead of using syslog to test the result, http-after-response rules
are now used. It is a bit simple. In addition, all custom timeouts are
tested. Some testcase were useless and were thus removed.
In addition, the script was moved in "reg-tests/http-rules/" directory.
Timeouts may be overwritten via the action "set-timeout". Client timeout is
supported on frontends, connect/queue/tunnel/server timeouts are supported
on backends and tarpit timeout is supported on both.
However, there is an issue with the listener. In that case, the action is
always executed in the frontend context and the backend-side timeouts are
then reset, when the backend is selected. If it is another backend, it is
logical. But most of time, the backend is unchanged. And in that case, these
timeouts must be preserved.
Documentation of set-timeout action was improved.
This patch should be backported to all supported versions. Be carefull, all
timeouts are not supported in 3.3 and lower (3.3->3.0: server/tunnel/client
- 2.8->2.6: server/tunnel)
When an HTX header or an HTX start-line is added or updated, a test is now
performed to be sure the whole headers size, including the start-line never
exceeds the regular buffer size. It is mandatory because it is now possible
to use larger buffers. However, it remains impossible to send HEADERS frame
in H2 and QUIC larger than a regular buffer. So we must be sure the HTTP
analysis will never produce too big headers because they will be rejected
later, at the forwarding stage. The purpose of large buffers is to be able
to store large payload, larger than regular buffers. Headers must remain
quite small.
This commit depends on the following one:
MINOR: htx: Add a field to save the headers data size
Both should be backported to 3.4 to avoid any trouble with large buffers. It
is not strictly speaking a bug, but it will avoid hazardous behavior
depending on the payload size.
The size of headers present in an HTX message are now counted. A dedicated
field was added in the HTX structure to do so. This patch is quite simple
but it will be mandatory to be able to perform some tests on headers. One of
them is to be sure headers never exceed the regular buffer size, even when a
large buffer is used.
All calls to get_trash_chunk_sz() and alloc_trash_chunk_sz() were reviewed
to be sure we never retrieve a large chunk when it is not expected. Some
calls were not upgrade, but several calls now rely on get_best_trash_chunk()
and alloc_best_trash_chunk() functions.
The idea of the fix is to get a large buffer only when the original buffer
is already a large buffer and the expected size of data exceeds the size of
a regular buffer. This should prevent unexpected memory usage.
This commit relies on the previous one:
MINOR: chunk: Add function to get a large/regular chunk depending on a buffer
Both must be backported to 3.4.
get_best_trash_chunk() function was added to be able to get a large or a
regular chunk depending on a given size but never larget than a given
buffer. It will be usefull in a futur fix, to prevent unexpected large chunk
usage.
alloc_best_trash_chunk() function is similar but instead of returning one of
the static chunks, it allocate it from the corresponding pool, large or
regular.
For a chunk size exactly equal to bufsize/2, the "max_size" variable could
underflow if a separator is provided. Indeed, "max_size" is first set to
(trash->size - 2 * chunk_size), so to 0. And on the first iteration, the
separator length is removed, making it to underflow.
On 3.3 and lower it is especially an issue with samples larger than
bufsize/2 because data could be written ouside of the buffer. On 3.4 and
3.5, it is not an issue because we fail to retrieve a chunk.
This should be backported to all supported versions.
The proxy used to parse healthcheck sections was not inserted in the proxies
list. So its initialization was not properly finalized. Among other things,
it was an issue for some arguments, like regular expressions.
This proxy is now inserted in the proxies list.
This patch should fix the issue #3440. It must be backported to 3.4.
When encountering an unexpected escape sequence, treat it as a literal
character instead of skipping it.
This is a deliberate choice for two reasons:
- to avoid a desynchronization between the h->data counter and the
buffer content, which would otherwise leave uninitialized memory
("garbage") in the destination buffer;
- to ensure that an invalid configuration string triggers a parsing
error immediately (fail-fast), rather than resulting in a silently
malformed configuration.
Thank to @dirkmueller for having reported this issue.
No need to backport. huf arrived with this current version.
When a stream is freed, hld_strm_task() was unconditionally re-queuing
the user task, even when no streams were left or when no update was
needed. This caused spurious task wakeups, which could incorrectly inflate
connection and request rate counters to come for the -R option implementation.
Only queue the user task if there are remaining streams to process, and
properly update the expiration time.
No need to backport. haload arrived with this current version.
Update the README to cover the new syncing feature: the deployment
section explains the three optional pieces and their relations. We
also explain the special cases of "Get updates" and "Save updates"
when combined with pending/conflicting changes (particularly in
edition mode).
An explanation of the difficulties to set git permission is also
provided.
Every save used to be committed as a bare "update 3.5", which makes the
storage history useless to navigate. The subject now names the branch
and the first commit whose review was touched, followed by "+ N more"
when several, and the body lists all of them one per line:
update 3.5: 6a7b27a0 + 2 more
6a7b27a0
d13aaf05
b12dd0b5
This is what to grep for when hand-editing the storage repository, for
example to locate the change to revert or rebase. A commit touched by
several directives of the same save (state plus notes) is only listed
once.
thttpd forwards the CGI's stderr to the client *ahead* of its stdout:
the socket receives the HTTP status line, then any stderr log line, and
only then the CGI headers, turning the log line into a bogus response
header; on an error path the same mechanism could push garbage in front
of the "Status:" header and corrupt the response entirely.
All diagnostics are already carried by the response itself (die()'s
body, the in-band "warning:" line with git's captured error), so the
duplicated stderr logging brings nothing and only risks breaking the
channel it leaks into: drop it, and state the constraint in a comment
above die() so it doesn't come back. The usage text for a bad command
line is folded into the 500 response body, which is also what a shell
user sees when testing by hand.
The "git commit failed" warning said nothing about the cause, leaving
the admin to guess between a missing identity, an ownership refusal, a
git binary absent from the restricted PATH the web server gives to its
CGIs, etc. run_git() now captures the command's stdout and stderr
through a pipe and the warning line carries git's own message (first
255 bytes, control chars flattened), so the admin directly sees the
cause; as the command runs through /bin/sh, an unfindable git yields
status 127 and the shell's message, reworded as "cannot execute git:
..." to directly point at the typical PATH issue. Capturing also
guarantees that git output can never corrupt the CGI response nor leak
to the client on servers which wire the CGI's stderr to the socket.
When the git commit fails after a save (typically a missing committer
identity in the storage repository, or an ownership/permission issue),
the failure was only logged to stderr, which lands in the web server's
error log at best: the file kept being updated but the history silently
stopped being recorded. Report it as a "warning: git commit failed ..."
line appended to the response, where the page can show it to the user,
on top of the stderr log. Also stop treating a no-op as a failure:
re-pushing identical content stages nothing, so the commit is now
simply skipped when "git diff --cached --quiet" reports no staged
change, instead of letting "git commit" fail on an empty commit.
Notes are append-only on the wire, which makes concurrent edits
conflict-free but leaves no way to revise or clean up a note from the
page: fixing a note requires a hand-edit of the storage file. This adds
the replacement directive that the design had reserved:
<cid> setnotes <hash> <replacement text>
Unlike the other directives, a replacement must carry a token of the
base it was computed from, or it could silently destroy a concurrent
update (one reviewer's append landing between another's read and
replace). The token is the SDBM hash (8 hex chars) of the note blob
the client based its edit on: the server only applies the replacement
if it still matches the stored blob (empty-string hash for a commit
without notes). On mismatch the directive is dropped, the line is left
exactly as found, and a "conflict <cid>" line is emitted in the
response before the resulting lines so that the client can point the
user at what needs manual reconciliation; other directives from the
same POST are still applied, and nothing is written nor committed when
everything conflicted. SDBM is trivially computed on both sides (a
concurrency token, not a security feature, and JS crypto is unavailable
on plain http anyway), and its small multiplier keeps the whole hash
computation exact in awk's double-precision arithmetic, which is
precisely why it was chosen over wider-multiplier hashes.
An empty replacement deletes the notes (and the line if no state is
left), finally allowing obsolete notes to be removed without editing
the file by hand. Replacements are capped to 4000 chars instead of the
500-char append cap, since a coalesced blob may legitimately have grown
beyond a single addition.
This adds the read side of the review persistence CGI: GET
update.cgi?branch=X.Y now returns the current overlay for that branch
as a JSON array of {"cid", "state", "notes"} objects with absent fields
omitted; a missing or empty file yields "[]". The raw storage format
never travels: the notes are unescaped by the parser and JSON-escaped
on output, so the client can JSON.parse() the response and insert the
notes via textContent directly. Unparseable lines or fields are
silently skipped as everywhere else.
Reads are lockless: the atomic rename on the write side guarantees
that the file is always a complete valid version. The response carries
Cache-Control: no-store so that a browser never reuses a stale overlay
on refresh.
The backport review page keeps the human edits (verdict overrides and
notes) only in the loaded DOM: they are lost on reload and never shared
between reviewers. This adds the server side of the shared persistence
design: update.awk, a GNU awk CGI script which stores these edits into
one file per major branch (e.g. "3.5") inside a dedicated git
repository, one line per touched commit:
<commit_id> [state <n|u|w|y>] [notes "<quoted notes>"]
The overlay only ever stores human edits keyed on the commit id; the AI
verdict and explanation stay in the generated HTML. Commit ids are
length-agnostic and matched by symmetric prefix (first match wins), so
the current 8-char pipeline and a future 12-char one both work without
any migration.
POST applies line-oriented directives ("<cid> state <n|u|w|y|revert>",
"<cid> notes <text to append>"), none of which carries a base value:
states are last-write-wins and notes are append-only (capped to 500
chars per push and sanitised so that no newline may ever enter a stored
line), which keeps concurrent edits conflict-free. Broken directives,
fields or lines are silently ignored, never fatal, and lines not being
modified are preserved byte-for-byte so that admin hand-edits survive.
Writers are serialised by a mkdir lock at an obvious place (<repo>/lock)
with PID-gated crash takeover via atomic rename, the file itself is
replaced by an atomic rename from a temp file inside the lock dir, and
every resulting state is committed to git, which acts as the event log
(git blame/log -L provide the full history). A crash at any point leaves
at worst a stale lock (reclaimed on the next write) or a valid but
uncommitted tree (folded into the next commit), never a broken file.
The takeover races are covered: the staleness decision and the takeover
rename are not one atomic operation, so the thief verifies after the
rename, discarding the stolen dir only if it still carries the pid that
was judged dead and renaming it back in place untouched otherwise; the
victim redoes the whole locked cycle from a fresh read when its final
rename fails, since nothing was applied to the branch file yet; a writer
finding its own pid in the lock adopts it as stale; and the release only
removes the lock after checking that it still contains our own pid.
A few awk specifics are worth noting: external commands (git, mkdir,
mv) go through /bin/sh, so everything interpolated into a command line
is shell-quoted (single-quote is escaped and the argument placed inside
single quotes); the -b (bytes) flag keeps all string operations byte-
based regardless of the locale; and a manual argv parsing due to gawk
silently consumes a leading "-r" argument, ignoring ours.
Also note that gawk uses a file cache for getline() and co, which opens
lots of traps so we need to be extremely careful about properly closing
files if we want to check for changes (e.g. lock's pid file).
Finally, writing through a redirection whose target cannot be opened is
a fatal awk error terminating the script without even a response, so the
writes into the (stealable) lock dir are arranged to resist a takeover:
the pid is written through the shell, where a vanished dir is a plain
command failure, and the temp file is opened the very instant the lock
is acquired, its descriptor surviving a later theft. The update.cgi
wrapper considers any >0 return code as a failure and returns a generic
error as it will indicate that the awk script itself couldn't produce
a valid response.
For now, only the POST ("save changes") action is implemented.
The "Get updates" and "Save changes" buttons only existed at the top
right of the page, while a review session ends at the bottom of the
table: with no button left in sight there, it was way too easy to
forget to save the work. Emit a copy of both buttons and of the status
line at the bottom right, sharing the same handlers; the status
message and the save-button graying are applied to both instances at
once so the two spots always tell the same story.
Some server setups leak the CGI's stderr into the response body (e.g.
inetd-style servers where fd 2 is the client socket): since stderr is
unbuffered and stdout is buffered, a git error message then lands
*before* the "OK <n>" line, and the applied-count check failed with a
cryptic "server applied only ? of N changes" although everything had
been applied. Scan the whole response for the lines of interest (the
"OK <n>" count, the "conflict" and "warning" lines) instead of assuming
they come first, dump the raw response to the console when no count is
found at all to ease diagnosis, and display the server's warning lines
(such as the new "git commit failed" one) next to the save status so
that a recording problem is visible from the page instead of being
buried in a server log.
The save handler treated any HTTP 200 as a full success and advanced
the local reference for everything it had sent, only special-casing the
reported conflicts. But the server legitimately drops directives it
cannot parse, and answers "OK <n> directives applied" with what it
really did. The typical case is an outdated update scripton the server
which ignores the whole "setnotes" directive, applies nothing, and
the client still displayed the edit as saved... until the next "Get
updates" reverted it.
Let's count the directives sent, and when the server's applied count
plus the reported conflicts don't add up, believe the server, not
ourselves: advance nothing, keep every edit local (boxes open, save
button lit) and tell the user how many changes were ignored,
suggesting a version mismatch.
The "Save changes" button used to remain active all the time, giving no
hint about whether anything was pending. It is now disabled whenever
nothing differs from the reference: no verdict change, no non-empty
note addition, no note edition differing from its base. It gets
re-evaluated after every action which may change that (verdict clicks,
typing in a note input, opening/cancelling a box, updates and saves),
bailing out at the first pending change so the common case stays cheap.
As a side effect, the button lighting up right after a reload confirms
at a glance that the browser restored unsaved local edits.
An "[edit note]" link now appears next to "[add note]" whenever a line
has shared notes: it presents the whole note blob in the input box for
edition, and the save sends it as a replacement (the "setnotes"
directive, carrying the hash of the blob the edit was based on).
Emptying the box deletes the note. Clicking "add note" first and then
"edit note" merges the reference notes with the pending addition so
nothing typed so far is lost, and a "[cancel]" link aborts an edition
opened by mistake without touching anything.
An open input only disappears once its content is synchronized with the
reference: a successful save, an update proving an exact match, or an
explicit cancel. To that end, "Get updates" first silently closes the
no-op boxes (opened but nothing changed), then after applying the
fetched state it closes the boxes it made redundant (an addition
someone already pushed, an edition matching the current notes), and
re-bases an edition whose base moved, turning it red: red uniformly
means "the reference changed under your edit, review the notes above
against your text before saving". A save refused by the server (the
"conflict <cid>" response lines) turns the input red the same way,
keeping it in edition; states and additions from the same save are
unaffected. After a reload, a browser-restored pending note reopens in
append mode, the only safe assumption since a lost replacement base
cannot be recovered.
This is the write side of the review syncing: a "Save changes" button
next to "Get updates" collects the local edits and pushes them to
update.cgi. An edit is a radio button change differing from the
reference state (so clicking around and coming back to the reference
sends nothing), or a note typed in the per-line input revealed by the
new "[add note]" link under the AI explanation (500 chars max, matching
the server-side cap).
No directive carries a base value: states are last-write-wins and notes
are append-only server-side, so two reviewers saving concurrently
cannot conflict. On success the reference advances to the pushed values
and the note inputs are cleared, so the page is clean without needing a
refetch; on error (server busy or unreachable) everything stays local
and a later click simply retries.
This adds the read side of the review syncing to the generated page: a
"Get updates" button at the top right retrieves the shared state from
update.cgi (reached by a bare relative URL, so it must be in a cgi-bin
directory next to the page) and applies it. Nothing is fetched
automatically, not even at load time: it's up to the user to explicitly
click to resynchronize, and without it (or with the server down) the
page keeps behaving fully standalone as today.
Three states exist per line to make this work. The original state is
the verdict the bot chose, captured at load time and constant. The
reference state is the last known shared state, on top of which the
user's edits sit; it starts equal to the original. The local state is
the DOM itself (the checked radios). Applying a fetched overlay
recomputes every line's reference as "the server's entry if any,
otherwise the bot's verdict", so a removed override properly falls back
to the original; the reference always advances but the displayed state
only moves where the user had no local edit: local edits win, and
re-applying the same overlay twice changes nothing.
The received commit ids are resolved exactly first, then by symmetric
prefix (one id being a prefix of the other, for mixed-length ids), first
line wins. The shared notes land in a dedicated container below the AI
explanation, rendered via innerText (no HTML injection) and replaced
wholesale so the operation stays idempotent; entries for commits absent
from the page are simply ignored.
The whole exchange was tested with the real generated page's scripts
running against a stubbed DOM covering load capture, prefix resolution,
adopt-vs-keep on both changed and disappeared entries, idempotent
re-application, and silent degradation on fetch failure.
The review page will need to exchange its state with the update.cgi
service sitting next to it, and for this it must know which branch it
covers since all branches' pages may share a directory. post-ai.sh
takes a new "-v <version>" argument and emits it as a "branch" JS
variable; when absent the variable is empty and the page will simply
not offer syncing, keeping the output standalone as today.
update-3.0.sh deduces the version from its own name (update-3.0.sh ->
3.0), so that adding a symlink with another version for a new branch
continues to work with no other change, and passes it to post-ai.sh.
The commit id column doesn't need to show more than 8 chars to stay
unambiguous within a single page, and longer ids needlessly widen the
table. Everything that is keyed on the id (the commit link, the row's
name= attribute and the cid[] JS array) keeps the full id produced by
the pipeline, whatever its length, so this is a display-only change
which also gets us ready for a possible future move of the pipeline to
longer ids.
When the page is reloaded, the browser restores the "review" radio
column to the user's last selection (e.g. "All"), but the "review" JS
variable is regenerated to the default first line to review: the
listing then restarts from that line while "All" still appears
selected, and one has to click a random line then "All" again to
really see everything.
Give an id to each line's review radio and resynchronize the variable
from the actually checked radio when the page loads: a restored "All"
(or any restored line) now behaves as selected, and on a fresh load the
checked radio is the generated one so nothing changes.
Several QUIC related keywords were removed in 3.4. The legacy options
were marked as deprecated and scheduled for removal in 3.5. This patch
applies this removal for the upcoming 3.5.
Commit 061754b249 attempted to make it
possible to close file descriptors belonging to other thread groups by
using thread isolation.
The problem is, closing other thread groups' fds usually happens when we
destroy a listener, in which case we hold the listener lock. If any
other thread tries to get that lock while we're waiting for the thread
isolation, then they will deadlock.
This can happen with any type of listener, but it is easier to reproduce
with a suspend/resume loop with ABNS sockets, as a suspend translates to
a close here.
To fix that, instead of using thread isolation, do something similar to
what's done when the fd belongs to our thread group. Increase the tgid
ref counter, so that we're sure nobody will close the fd while we're
dealing with it, then set the FD_MUST_CLOSE bit and set thread_mask to
0.
At this point, if no thread was running on that fd, no one will and we
can safely close it. So just call _fd_delete_orphan() if the
running_mask is 0 and if the FD_MUST_CLOSE bit is still there, otherwise
we can safely assume another thread will take care of it.
This should be backported up to 2.8.
For frontend connections, quic_conn layer is able to reject any new
streams opened after MUX closure. This is necessary as the peer may not
have been notified yet of the closure.
This operation is unnecessary on backend side. This is due to the fact
that only HTTP protocols are currently supported on top of QUIC, with
requests initiated by the client. For requests started before the MUX
closure, either they are already completed or closed early with a
STOP_SENDING emitted during stream shut.
Prior to this patch, spurrious RESET_STREAM could have been emitted on
backend connections after MUX closure as quic_conn stream_max_bidi was
not correctly set. Now reject is only performed for frontend connections
so this should not occured anymore.
This should be backported up to 3.3.
Prior to this patch, shut stream callback only handles write channel
closure. In case of an early closure, a RESET_STREAM would be emitted.
On the frontend side in most cases this is sufficient as read channel is
already closed, as HTTP/3 GET requests has been fully received. However,
this may not be the case for POST requests. Also, on the backend side,
haproxy acts a client. In this case, a stream early closure will
typically happen before receiving the full response. Nothing will be
emitted (RESET_STREAM is unnecessary as write channel is already
closed), thus the server peer will continue to emit.
To fix this situation, the current patch implement read channel closure
on shut if SE_SHR_RESET is set. Callback lclose from app_ops is called
with a new dedicated mode for read channel closure, which will result in
a STOP_SENDING frame generated by H3 and hq transcoders. This will
instruct the peer to stop emission.
This should be backported up to 3.3. Note that this depends on the
following patch :
dde3ee06c30f20091443bdafdda0e0294f7ac26b
MINOR: mux_quic: use separate error code for STOP_SENDING
