From 9b1b2c760f73827fc08ade3a936a89fa5473f8fa Mon Sep 17 00:00:00 2001
From: Caio Ramos Casimiro <caiorcasimiro@gmail.com>
Date: Wed, 9 Oct 2024 15:00:06 +0100
Subject: [PATCH] fix(ffi) correct usage of 'n_bins' when parsing a histogram

Previously, when parsing a histogram without uninitialized bins,
`parse_cmetric` would incorrectly build an extra bin from
invalid memory.

Histograms with uninitialized bins were correctly parsed due to
`parse_cmetric` stop parsing bins when an uninitialized one is found.
---
 lib/resty/wasmx/shm.lua                       |  2 +-
 .../ffi/shm/024-metrics_get_by_name.t         | 23 +++++++++++++------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/resty/wasmx/shm.lua b/lib/resty/wasmx/shm.lua
index c4da9e8cf..e0da2e6da 100644
--- a/lib/resty/wasmx/shm.lua
+++ b/lib/resty/wasmx/shm.lua
@@ -438,7 +438,7 @@ local function parse_cmetric(cmetric)
         local ch = ffi_cast("ngx_wa_metrics_histogram_t *", hbuf)
         local h = { type = "histogram", value = {}, sum = tonumber(ch.sum) }
 
-        for i = 0, ch.n_bins do
+        for i = 0, (ch.n_bins - 1) do
             local cb = ch.bins[i]
             if cb.upper_bound == 0 then
                 break
diff --git a/t/04-openresty/ffi/shm/024-metrics_get_by_name.t b/t/04-openresty/ffi/shm/024-metrics_get_by_name.t
index c3e4fa3de..95df115b2 100644
--- a/t/04-openresty/ffi/shm/024-metrics_get_by_name.t
+++ b/t/04-openresty/ffi/shm/024-metrics_get_by_name.t
@@ -47,7 +47,19 @@ h1: {sum=100,type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
 === TEST 2: shm_metrics - get_by_name() sanity non-FFI-defined metrics
 prefix: pw.hostcalls.*
 --- wasm_modules: hostcalls
---- config
+--- config eval
+my $record_histograms;
+
+foreach my $exp (0 .. 17) {
+    my $v = 2 ** $exp;
+    $record_histograms .= "
+        proxy_wasm hostcalls 'on_configure=define_metrics \
+                              test=/t/metrics/record_histograms \
+                              metrics=h1 \
+                              value=$v';";
+}
+
+qq{
     location /t {
         proxy_wasm hostcalls 'on_configure=define_metrics \
                               on=request_headers \
@@ -60,11 +72,7 @@ prefix: pw.hostcalls.*
                               test=/t/metrics/toggle_gauges \
                               metrics=g1';
 
-        proxy_wasm hostcalls 'on_configure=define_metrics \
-                              on=request_headers \
-                              test=/t/metrics/record_histograms \
-                              metrics=h1 \
-                              value=100';
+        $record_histograms
 
         content_by_lua_block {
             local shm = require "resty.wasmx.shm"
@@ -75,10 +83,11 @@ prefix: pw.hostcalls.*
             ngx.say("h1: ", pretty.write(shm.metrics:get_by_name("pw.hostcalls.h1", { prefix = false }), ""))
         }
     }
+}
 --- response_body
 c1: {type="counter",value=13}
 g1: {type="gauge",value=1}
-h1: {sum=100,type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
+h1: {sum=262143,type="histogram",value={{count=1,ub=1},{count=1,ub=2},{count=1,ub=4},{count=1,ub=8},{count=1,ub=16},{count=1,ub=32},{count=1,ub=64},{count=1,ub=128},{count=1,ub=256},{count=1,ub=512},{count=1,ub=1024},{count=1,ub=2048},{count=1,ub=4096},{count=1,ub=8192},{count=1,ub=16384},{count=1,ub=32768},{count=1,ub=65536},{count=1,ub=4294967295}}}
 --- no_error_log
 [error]
 [crit]