Skip to content

fix(logback): drop '--' from comment to fix SAXParseException at startup#69

Merged
VirusAlex merged 1 commit intomainfrom
fix/logback-xml-comment
May 1, 2026
Merged

fix(logback): drop '--' from comment to fix SAXParseException at startup#69
VirusAlex merged 1 commit intomainfrom
fix/logback-xml-comment

Conversation

@VirusAlex
Copy link
Copy Markdown
Owner

Bug

PR #65 added a date to the logback.xml timestamp pattern with an explanatory comment. The comment included the literal token kubectl logs --previous to motivate the choice. XML forbids the -- sequence inside comments — it terminates the comment parser early.

At every JVM startup since v0.4.0:

ch.qos.logback.core.joran.spi.JoranException: Problem parsing XML document. ...
Caused by: org.xml.sax.SAXParseException; ... line 9 column 46;
  The string "--" is not permitted within comments.

Logback fell back to its default appender (no pattern, no level overrides) and the app continued with degraded logging. Reported by VirusAlex on a v0.4.1 live docker pull test.

Why it slipped through

  • ArchUnit doesn't validate XML.
  • The only mvn-test path that exercises logback init is the Jetty-using server tests, which fail under our Windows CI env for unrelated loopback reasons.
  • The CI build-and-package step packages the jar but never actually starts the JVM with logback.xml on the classpath.
  • Local mvn test uses logback's default config, never reads our resource.

In short: no automated path even tries to load this file.

Fix

Replaced the verbose comment (which also did unnecessary scope creep into Kubernetes / journalctl framing — NetCopy targets two trusted hosts on a LAN, not k8s) with a tight one-liner that doesn't reference command-line flags.

-            <!--
-                ISO date in the timestamp so a `grep` across days isn't ambiguous
-                (HH:mm:ss alone repeats every 24h). Container log consumers like
-                Docker's json-file driver also stamp lines with the host's wall
-                clock, but those timestamps don't survive when the user pipes
-                logs through `kubectl logs --previous`, journalctl exports,
-                or the GitHub Actions UI's run logs.
-            -->
+            <!-- ISO date in the timestamp so `grep` across days isn't
+                 ambiguous (HH:mm:ss alone repeats every 24h). -->

Verification

  • xmllint-equivalent validation: XML parses cleanly via .NET XmlDocument.
  • java -jar netcopy.jar --version starts with no SAXParseException and prints version line.
  • CI green.

Follow-up

After this lands I'll move the v0.4.1 tag onto the new HEAD so the GHCR / GH Release artifacts are rebuilt with the working logback config. The currently-published :0.4.1 works (logback's default appender prints lines, just without the configured pattern), but it'll keep emitting that JoranException stack trace at every startup until users repull. Force-moving the tag is fine here — the release is a v0.x pre-release and only ~one user has pulled it.

🤖 Generated with Claude Code

… at startup

PR #65 added a date to the logback.xml timestamp pattern with a long
comment explaining why. The comment included the literal token
'kubectl logs --previous' to motivate the choice. XML forbids the
'--' sequence inside comments — and rightly: it terminates the comment
parser early and ambiguates with the closing '-->'.

Effect at runtime: every JVM startup hit

  ch.qos.logback.core.joran.spi.JoranException:
    Problem parsing XML document. ...
  Caused by: org.xml.sax.SAXParseException; ... line 9 column 46;
    The string "--" is not permitted within comments.

Logback then fell back to its default appender (no pattern, no level
overrides) and the rest of the app proceeded with degraded logging.
Reported by VirusAlex on the v0.4.1 docker-pull live test.

Why it slipped through: ArchUnit doesn't validate XML; the only smoke
test that exercises logback init was the Jetty-using server tests, which
fail under our Windows CI env for unrelated loopback reasons. The CI
build-and-package step also doesn't actually start the JVM with
logback.xml on the classpath. Local mvn-test on JDK 25 didn't trip it
because the test infra uses logback's default config.

Fix: rewrite the comment as a one-liner that doesn't reference command-
line flags. The kubernetes/GH-Actions framing was speculative scope
creep anyway — NetCopy targets two trusted hosts on a LAN, not k8s.

Verified locally: XML parses, java -jar netcopy.jar --version starts
clean with no SAXParseException.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VirusAlex VirusAlex merged commit 235ee75 into main May 1, 2026
1 check passed
@VirusAlex VirusAlex deleted the fix/logback-xml-comment branch May 1, 2026 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants