Implement HttpRequestAccessor for nested CEL field access - #711
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe changes introduce HTTP request accessor support to the CEL environment by defining a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
271a2e5 to
6bd65bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pkg/utils/cel.go`:
- Around line 47-53: The has(http.request.*) predicate (requestIsSet) only
checks x.Raw.HttpEvent != nil which is unreliable because CelEvent is always
wrapped; change the predicate to call the accessor (use x.GetRequest()) and
verify the returned request is non-nil and for URL/path checks also ensure
request.URL (or its string) is non-empty; update the same pattern in the other
predicates referenced (the similar blocks around the Request/URL checks) so
event.request presence gating uses GetRequest() and request.URL rather than just
HttpEvent != nil.
- Around line 513-534: The GetFrom closure for the "body" field in
pkg/utils/cel.go currently reads req.Body directly which consumes it; change the
read to a safe, limited-read and then restore req.Body so downstream readers
aren’t broken: in the ref.FieldGetter closure (inside the
xcel.Object[HttpRequestAccessor] handling and after req :=
x.Raw.HttpEvent.GetRequest()), read up to a reasonable size limit (e.g., via
io.LimitedReader) into a buffer, build a new io.ReadCloser from the buffer
(e.g., bytes.NewBuffer + io.NopCloser) and assign it back to req.Body, handle
errors from io.ReadAll appropriately, and return the string from the buffered
data; ensure you still prefer GetBuf() when present.
c78ff4b to
8f2f6d4
Compare
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
8f2f6d4 to
b05eaf1
Compare
|
cool, does it improve the pprof? |
Summary by CodeRabbit