Uh oh!
There was an error while loading. Please reload this page.
Pin the HTTP command server's behaviour before hardening it - #174
Merged
Conversation
Nothing tested the HTTP server, and it is due for changes to request parsing and path resolution. These seven cases record what it does today, so a regression shows up here rather than as a blank map in a browser. Two behaviours the web console depends on and that any hardening must preserve: - POST / with a body of "ff:<command>" reaches the interpreter. That is the entire UI-to-engine channel (forefireGUI.js sendCommand). - Files resolve against the working directory. The console issues plot[filename=fuel.png], ForeFire writes fuel.png beside the case, and the browser fetches /fuel.png as a static file. A document root that does not cover both htdocs and the working directory would break every overlay. Deliberately not covered: path traversal. Asserting the current behaviour would document an unfixed weakness in a public repository. Its test belongs with the fix, when the assertion can be that the request is refused. Two problems found while writing this, neither fixed here: - HttpCommandServer.hpp was not self-contained. It used std::ifstream without including <fstream> and an unqualified cout, so it compiled only when included after a header that supplied both. Fixed, because the test cannot include it otherwise; no behaviour change. - stop() deadlocks. It closes the listening socket and then joins the accept thread, but closing a socket does not reliably wake a thread blocked in accept() on Linux, so the join never returns. The suite therefore starts one server and never stops it. Fixing that needs shutdown() or a self-pipe and belongs with the hardening work.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nothing tested the HTTP server, and it is due for changes to request parsing and path resolution. These seven cases record what it does today, so a regression shows up here rather than as a blank map in someone's browser.
Purely additive: no change to how the server behaves.
What the web console depends on
Both are covered, and any hardening has to preserve them:
The
ff:endpoint is the entire UI-to-engine channel.forefireGUI.jssendCommand()posts to/with a body offf:<command>. It cannot be removed or authenticated away without redesigning the console.Files resolve against the working directory.
buildPlotCommandemitsplot[parameter=fuel;filename=fuel.png;...], ForeFire writesfuel.pngbeside the case, and the browser then fetches/fuel.pngas a static file. A document root confined totools/htdocswould break every map overlay — this is the trap most likely to be hit by a well-intentioned path fix.Cases
ff:reaches the command callback/ff:reaches the command callbackDeliberately not covered: path traversal
A test asserting the current behaviour would document an unfixed weakness in a public repository. That case belongs with the fix, when the assertion can be that the request is refused.
Two problems found while writing this, neither fixed here
HttpCommandServer.hppwas not self-contained. It usedstd::ifstreamwithout including<fstream>, and an unqualifiedcout, so it compiled only when included after a header that happened to supply both —Command.hwas the only such place. Fixed by adding the include and qualifyingcout, because the test cannot include the header otherwise. Compile-only; no behaviour change.stop()deadlocks. It setsrunning = false, closes the listening socket, then joins the accept thread — but closing a socket does not reliably wake a thread blocked inaccept()on Linux, so the join never returns. My first version of this suite hung for exactly this reason.The suite therefore starts one server and deliberately never stops it; the reasoning is in a comment so nobody "tidies" it later. Nothing has noticed until now because the server is never stopped in normal use — it runs until the process exits. Fixing it needs
shutdown()or a self-pipe, which is a change to server behaviour and so is deferred with the rest of the hardening.Verification
ctest4/4.--order-by=randacross three seeds.runffunchanged — KML and NetCDF both within tolerance.Drafted by Claude Opus 5. Reviewed by a maintainer before filing.