-
-
Notifications
You must be signed in to change notification settings - Fork 89
Introduce Spicy parser framework with HTTP parser implementation #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: gsoc2025/issue-184
Are you sure you want to change the base?
Introduce Spicy parser framework with HTTP parser implementation #186
Conversation
* Adds C++ bridge (bridge.{h,cpp}) to embed Spicy/HILTI in Glutton
* Spawns single-threaded worker goroutine for parser calls
* Registers and exposes parsers via spicy.Initialize()
* Implements Spicy-based HTTP handler and grammar (parsers/http.spicy)
* Adds wire protocol selector in protocols.go (toggle with viper `spicy.enabled`)
* Adds build rules for Spicy grammars (protocols/spicy/Makefile)
…ze cap & init errors
* reserves field slots without duplicating names; avoid double-free / leak * detects and aborts on strdup / malloc failures, rolling back field_count * adds early-exit helper * adds clarifying comments
|
@glaslos @furusiyya Here is my PR for the protocol parser implementation. I believe the GitHub action will need to be changed to accommodate the Spicy runtime. I've left We might need to include some documentation about the installation of the Spicy runtime and clang too. |
|
@glaslos @furusiyya I have sorted out the GitHub actions checks too. The checks are passing successfully after including the Spicy runtime in the actions runner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the Spicy HTTP parser and its Go/C++ integration to optionally replace the existing pure-Go HTTP parser when enabled in configuration. Key changes include:
- Exporting and using the embedded TCP resources FS as
Resinstead ofres - Introducing the full C++ bridge layer and Go worker thread model for Spicy parsers
- Wiring up Spicy initialization, shutdown, and TCP handler selection via a Viper config flag
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| protocols/tcp/resources.go | Export embedded FS as Res |
| protocols/tcp/http.go | Switch to using Res.ReadFile for embedded files |
| protocols/spicy/parsers/http.spicy | New Spicy HTTP grammar |
| protocols/spicy/parser.go | Go worker model and generic parse/cleanup functions |
| protocols/spicy/helpers.go | Utility functions for nested flat maps |
| protocols/spicy/handlers/http.go | Spicy-based HTTP handler, mirroring existing behavior |
| Makefile | Updated build command to use Clang and dropped LDFLAGS logic |
Comments suppressed due to low confidence (2)
protocols/spicy/helpers.go:82
- This helper is marked unused and lacks tests. Either remove it or add unit tests to ensure it behaves correctly when converting flat maps to nested structures.
func NestedFromFlat(flat map[string]interface{}) map[string]interface{} {
protocols/spicy/handlers/http.go:84
- The pure-Go handler used
strings.Contains, matching URIs with query parameters. Switching toHasPrefixmay miss valid version endpoints (/v1.16/version?foo=bar). Consider reverting toContainsor explicitly handling query strings.
if !strings.HasPrefix(uri, "/v1.16/version") {
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
@glaslos I've resolved all of Copilot's concerns; they were fairly minor. I hope the overall structure of the implementation is fine. It would be great if you could go through |
glaslos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial thoughts.
|
@glaslos just for your info: Mid-term Review Meeting Minutes: July Week 5 Sync: |
|
@furusiyya what is needed to get this over the finish line? In case @Boolean-Autocrat is not available anymore, I can get the PR ready to be merged. |
|
@glaslos I was waiting for @Boolean-Autocrat to push spicy DNS parser but he is not available. I will try to get this over the finish line in next month. |
Implements 2/3 of #184
This PR introduces Spicy protocol parser integration with C++ into Glutton, along with a concrete Spicy HTTP parser and handler which mimics the existing functionality of the pure-Go HTTP parser.
Spicy parsers can be enabled via Viper config (
spicy.enabled: true) and will seamlessly replace the Go parsers for the supported protocols (only HTTP for now).Architecture
C++ Bridge Layer (
protocols/spicy/bridge.{h,cpp})The core integration uses a C++ bridge that interfaces between Spicy/HILTI runtimes and Go:
spicy_parse_generic()accepts any parser name and raw dataGo Worker Thread Model (
protocols/spicy/parser.go)This was a fairly important design decision I had to make. Due to Spicy/HILTI's thread-local storage requirements, I decided that parsing should operate through a dedicated worker:
runtime.LockOSThread()workerCmdmessagesKey C++ Parsing Function
The
dump_value()function recursively processes HILTI's type system:field[0],field[1], etc.prefix.keyprefix.fieldnameImplementation
The new HTTP grammar file (
protocols/spicy/parsers/http.spicy) shows the integration of Spicy into the Glutton system. I've also written a corresponding handler (protocols/spicy/handlers/http.go) which tries to maintain behavioral compatibility with the existing pure-Go HTTP handler (while leveraging Spicy under the hood).As for the integration points, in
protocols/protocols.goI have added a simpleviper.GetBool("spicy.enabled")within the "poor man's detection" for HTTP and the entire Spicy runtime management (init and shutdown) happens inglutton.goafter checking the same Viper boolean.Adding New Parsers
I have tried to keep the addition of new parsers as seamless as possible. The flow looks something like this:
.spicyfile inprotocols/spicy/parsers/make spicyautomatically compiles Spicy grammars to C++protocols/spicy/handlers/Other than the usual change for registering in
protocols/protocols.goeverything can basically remain untouched. The only change required inprotocols/spicy/bridge.cppwill be the addition of an include statement for the new parser's generated header file.Notes
.ccor.hfiles generated by Spicy. In the main Glutton directory, you can simply runmake spicywhich will trigger their creation.protocols/spicy/parser_test.goandprotocols/spicy/handlers/http_test.gowhich verify the workings of both the parser and the handler with some basic edge cases.Screenshots (showing the Spicy HTTP parser in action)
Commands ran: