Skip to content

execute_request can be left without a reply: unguarded substring in the compilation-error formatter #125

Description

@jicoopdev

Summary

When JJava formats a CompilationException, StringStyler.highlightSubstringLines calls
String.substring(start, end) with positions taken straight from a javac Diag. The caller
checks only that both positions are non-negative — never that start <= end. A Diag whose
endPosition is smaller than its startPosition therefore throws
StringIndexOutOfBoundsException inside the error-reporting path itself, and the
execute_request that triggered it never gets an execute_reply. The client is left waiting
on a request that will never complete.

I should be upfront about the evidence: the mechanism is verified in the source and I have one
live occurrence with the kernel's own log line, but I have not been able to characterise a
reliable trigger.
Negative results are listed below. I'm filing anyway because the failure mode
(a silently lost reply) is bad out of proportion to how rare it is, and because the guard is a
one-liner independent of whatever produces the malformed Diag.

Environment

  • JJava 1.0-a7 (kernel banner: Java 21.0.12+8-LTS :: JJava 1.0-a7 :: Protocol v5.3)
  • Also verified still present on main @ db31f2c
  • JDK 21.0.12+8 and 25.0.4+7 (Temurin), Linux x86_64
  • Reached over the plain Jupyter ZMQ protocol via jupyter_client

Mechanism

The throwing statement, StringStyler.java line 170 — identical line number at 1.0-a7 and
on main:

// jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/util/StringStyler.java:170
hlLine.append(this.highlight(src.substring(start, Math.min(srcLineBreak.start(), end))));

The caller guard, JavaKernel.java — line 142 at 1.0-a7, line 144 on main:

// jjava-kernel/src/main/java/org/dflib/jjava/kernel/JavaKernel.java
if (d.getStartPosition() >= 0 && d.getEndPosition() >= 0)
    fmt.addAll(this.errorStyler.highlightSubstringLines(snippet.source(),
            (int) d.getStartPosition(), (int) d.getEndPosition()));

Non-negativity is checked; ordering is not. With start = 61 and end = 0,
substring(61, Math.min(_, 0)) becomes substring(61, 0) and throws.

Why the whole reply is lost rather than just the formatting:

  1. BaseKernel.handleExecuteRequest catches the CompilationException and, inside that catch
    block, calls env.publish(PublishError.of(e, this::formatError))BaseKernel.java:432 at
    1.0-a7, :431 on main.
  2. PublishError.of evaluates the formatter eagerly, at PublishError.java:19
    (List<String> stacktrace = formatter.format(exception);), i.e. while the argument to
    env.publish(...) is still being constructed. So the throw happens before anything is
    published.
  3. The next line — env.defer().replyError(ExecuteReply.MESSAGE_TYPE.error(), error),
    BaseKernel.java:433 / :432 — is never reached, so no execute_reply is ever queued.
  4. ShellChannel catches the escaping exception, logs it at WARN, and runs
    env.resolveDeferrals() in a finally (ShellChannel.java:64-73, same at both revisions).

The net effect on the wire: the deferred status: idle is published and the kernel stays
healthy and answers later messages, but the client receives neither the error message on
iopub nor the execute_reply on shell
. A frontend that waits for the reply to settle the cell
waits forever.

Observed

Seen once live, on JJava 1.0-a7, while sending a cell that referenced a class whose own defining
snippet had previously been rejected. The kernel logged exactly:

Unhandled exception handling execute_request. StringIndexOutOfBoundsException - Range [61, 0) out of bounds for length 76

and sent no reply; the client hung until its own timeout. The log line matches
ShellChannel.java:67-70's format string, and Range [61, 0) matches substring(61, 0) — a
Diag with startPosition = 61, endPosition = 0 against a 76-character snippet source.

What I could not reproduce

Honest negatives, so nobody re-runs them:

  • Ten candidate shapes — undefined-type constructor references in assignment and in argument
    position, generic-method arguments, and user-defined functional-interface parameters — all
    returned a clean CompilationException and a normal error reply.
  • A targeted probe of the shape the live occurrence suggested (define a class containing a
    syntax error so its snippet is REJECTED, then reference that class by name from a later cell)
    also did not reproduce it: both steps returned normal error replies, and a control statement
    afterwards confirmed the kernel was still responsive.

So: mechanism and one observed instance, reproduction unknown. I'd rather report that honestly
than dress up a repro I don't have. If it would help, I'm happy to run any candidate shape you
suspect.

Impact

Rare, but the failure is silent and unrecoverable from the client's side: no error output, no
reply, and the kernel looks idle and healthy the whole time, so the natural user response is to
restart it. It occurs specifically while reporting a compile error — i.e. only ever on a path the
user reaches by making a mistake.

Suggested fix

The narrow fix is to make the existing guard check ordering as well:

if (d.getStartPosition() >= 0 && d.getEndPosition() >= d.getStartPosition())

with the else branch already present (primaryLines(snippet.source())) as the fallback, which
degrades to an unhighlighted source listing rather than losing the reply. Clamping defensively
inside highlightSubstringLines would harden it wherever else that helper is called from.

Separately, and worth considering on its own merits: ShellChannel.java:64-73 currently turns
any exception escaping any handler into a WARN log and a lost reply. Replying with an error
in that catch block would convert this whole class of bug from "client hangs" into "client shows
an error", regardless of what threw.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions