|
| 1 | +# Analyse PR #3540 (SonarQubeCloud + JSON-Backend-Trennung) |
| 2 | + |
| 3 | +Datum der Analyse: 2026-04-13 (UTC) |
| 4 | + |
| 5 | +## Verifizierte externe Quellen |
| 6 | + |
| 7 | +- GitHub Pull Request: https://github.com/owasp-modsecurity/ModSecurity/pull/3540 |
| 8 | +- GitHub Checks API für Head-Commit `5dd7b3539977de85f0a2aebd2cb28b9ea1640211` |
| 9 | +- SonarCloud API (öffentlich): |
| 10 | + - `api/qualitygates/project_status?projectKey=owasp-modsecurity_ModSecurity&pullRequest=3540` |
| 11 | + - `api/measures/component?component=owasp-modsecurity_ModSecurity&pullRequest=3540&metricKeys=...` |
| 12 | + - `api/hotspots/search?projectKey=owasp-modsecurity_ModSecurity&pullRequest=3540` |
| 13 | + - `api/hotspots/show?hotspot=AZ2DWE24t-zbsGOGdN_L` |
| 14 | + |
| 15 | +## Relevante, lokal verifizierte Projektstellen |
| 16 | + |
| 17 | +- Backend-Selektion via Configure-Option und Compile-Defines: `configure.ac`. |
| 18 | +- Backend-spezifische Source-Auswahl in `src/Makefile.am`. |
| 19 | +- Adapter-Schicht und Ergebnis-Normalisierung in `src/request_body_processor/json_adapter.*`. |
| 20 | +- Gemeinsames Sink-Interface + Parse-Status in `src/request_body_processor/json_backend.h`. |
| 21 | +- JSON-Prozessor (Parser-Entry für Transaktion) in `src/request_body_processor/json.*`. |
| 22 | +- Backend-Implementierungen: |
| 23 | + - `src/request_body_processor/json_backend_simdjson.cc` |
| 24 | + - `src/request_body_processor/json_backend_jsoncons.cc` |
| 25 | +- Instrumentation in `src/request_body_processor/json_instrumentation.*`. |
| 26 | +- Matrix-Testskript über beide Backends in `test/run-json-backend-matrix.sh`. |
| 27 | +- Backend-spezifische Tiefe/Number-Lexeme-Tests in `test/unit/json_backend_depth_tests.cc`. |
| 28 | +- Regression-Edgecases in `test/test-cases/regression/request-body-parser-json-backend-edgecases.json`. |
| 29 | + |
| 30 | +## SonarQubeCloud: verifizierter Gate-Status |
| 31 | + |
| 32 | +`qualitygates/project_status` liefert: |
| 33 | + |
| 34 | +- `status=ERROR` |
| 35 | +- `new_security_hotspots_reviewed = 0.0` bei Schwellwert `>= 100` (Gate-Fehler) |
| 36 | +- `new_duplicated_lines_density = 1.3` bei Schwellwert `<= 3` (Gate **nicht** verletzt) |
| 37 | + |
| 38 | +Damit ist auf Basis der API der Quality-Gate-Fehler auf den nicht reviewten Security Hotspot zurückzuführen, nicht auf Duplikation. |
| 39 | + |
| 40 | +## Security Hotspot (verifiziert) |
| 41 | + |
| 42 | +- Datei: `test/benchmark/json_benchmark.cc` |
| 43 | +- Zeile: 318 |
| 44 | +- Regel: `cpp:S1313` („Using hardcoded IP addresses is security-sensitive“) |
| 45 | +- Meldung: „Make sure using this hardcoded IP address is safe here.“ |
| 46 | +- Sonar-Status: `TO_REVIEW`, `vulnerabilityProbability=LOW` |
| 47 | + |
| 48 | +Hinweis: Der Hotspot liegt in Test-/Benchmark-Code, nicht in den produktiven JSON-Backend-Dateien. |
| 49 | + |
| 50 | +## JSON-Backend-Trennung: verifizierte Architektur |
| 51 | + |
| 52 | +1. **Build-Time Auswahl (mutuell exklusiv)** |
| 53 | + - `--with-json-backend=simdjson|jsoncons` in `configure.ac`. |
| 54 | + - Definiert genau eines von `MSC_JSON_BACKEND_SIMDJSON` oder `MSC_JSON_BACKEND_JSONCONS`. |
| 55 | + - `src/Makefile.am` fügt abhängig davon genau eine Backend-Datei (`json_backend_simdjson.cc` oder `json_backend_jsoncons.cc`) hinzu. |
| 56 | + |
| 57 | +2. **Gemeinsame Laufzeit-Abstraktion** |
| 58 | + - Gemeinsames Event-/Status-Interface in `json_backend.h` (`JsonEventSink`, `JsonParseResult`, `JsonParseStatus`, `JsonSinkStatus`). |
| 59 | + - `JSONAdapter::parseImpl()` dispatcht per Compile-Makro auf genau ein Backend. |
| 60 | + |
| 61 | +3. **Gemeinsame Verarbeitungsschicht oberhalb der Backends** |
| 62 | + - `JSON` implementiert `JsonEventSink` und mappt Events auf `Transaction::addArgument(...)`. |
| 63 | + - Dadurch teilen beide Backends denselben Semantikpfad für Argument-Erzeugung/Depth-Handling im Sink. |
| 64 | + |
| 65 | +## Verifizierte Überschneidungen / Kopplungen |
| 66 | + |
| 67 | +- **Gewollte Kopplung über gemeinsames Sink-Interface**: beide Backends liefern identische Event-Arten an dieselbe `JSON`-Klasse. |
| 68 | +- **Build-Kopplung in Tests**: `test/Makefile.am` enthält `-I$(top_srcdir)/others/jsoncons/include` in allgemeinen Test-CPPFLAGS, auch wenn simdjson als Backend gewählt ist. |
| 69 | +- **Teilweise duplizierte Helper-Logik** zwischen Backends (z. B. `makeResult(...)`, `stopTraversal(...)`, ähnliche Event-Dispatch-Strukturen), jedoch jeweils mit backend-spezifischen Parser-APIs. |
| 70 | + |
| 71 | +## Sonar-relevante Open-Issues in JSON-Dateien (Auszug) |
| 72 | + |
| 73 | +- `json_backend_jsoncons.cc`: |
| 74 | + - hohe Cognitive Complexity (mehrere Funktionen) |
| 75 | + - Nesting-Tiefe > 3 in mehreren Blöcken |
| 76 | +- `json.cc`: |
| 77 | + - hohe Cognitive Complexity in `complete(...)` |
| 78 | + - Nesting-Verstöße in `complete(...)` |
| 79 | + - Rule zu manuellem `delete` |
| 80 | +- `json.h`: |
| 81 | + - Rule zu Copy-/Move-Semantik (`JSON` besitzt rohe Pointer in Containern) |
| 82 | +- `json_instrumentation.cc`: |
| 83 | + - Rule zu globaler Variable (`thread_local JsonInstrumentationMetrics g_metrics`) |
| 84 | +- `json_backend_simdjson.cc`: |
| 85 | + - `enforceTechnicalDepth(...)` sollte laut Sonar `const` sein |
| 86 | +- `json_adapter.cc`: |
| 87 | + - mehrere Minor-Hinweise zu `std::string_view`/`const`-Parametern |
| 88 | + |
| 89 | +## Minimal-invasive Refactoring-Ideen (direkt aus Code ableitbar) |
| 90 | + |
| 91 | +1. `json.cc::complete(...)` in kleine Mapper-Funktion für Fehlertext aufspalten. |
| 92 | +2. `json_backend_jsoncons.cc::emitEvent(...)` in Event-Handler-Funktionen splitten (`handleStringEvent`, `handleNumberEvent`, etc.). |
| 93 | +3. `json_backend_jsoncons.cc::consumeStringAt(...)` und `consumeNumberAt(...)` in kleinere Validator-Helfer extrahieren. |
| 94 | +4. `json.h/json.cc`: Ownership von `m_containers` auf `std::unique_ptr<JSONContainer>` umstellen, `delete` entfernen. |
| 95 | +5. `json_instrumentation.cc`: `g_metrics` in Funktion-scope kapseln (`metricsRef()`), Zugriffe zentralisieren. |
| 96 | +6. `json_backend_simdjson.cc`: `enforceTechnicalDepth(...) const` markieren (wenn API-Aufrufe const-korrekt bleiben). |
| 97 | + |
0 commit comments