Don't forward skip or only to aggregator

These two parameters are not idempotent, and are currently being run
first on the satellite, then again on the aggregator.  This results in
errors and unexpected results.

This commit detects when we're parsing a top collection, and filters out
those two parameters from being applied to the satellite request.

To accomplish rewriting the URI, a new Request API needs added for
non-const access to the URI object.

Tested: Aggregator shows results as expected.  Query params are not
forwarded to satellite

Change-Id: I99cbbb08da9fcd06c9ee10d371b253e32d01f59b
Signed-off-by: Ed Tanous <etanous@nvidia.com>

2 files changed, 34 insertions, 0 deletions

diff --git a/http/http_request.hpp b/http/http_request.hpp
index 7cf2842cb3..74681ccb7e 100644
--- a/http/http_request.hpp
+++ b/http/http_request.hpp
@@ -111,6 +111,11 @@ struct Request
         return req.target();
     }
 
+    boost::urls::url& url()
+    {
+        return urlBase;
+    }
+
     boost::urls::url_view url() const
     {
         return {urlBase};
diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp
index eb216500a1..5f2ac18a50 100644
--- a/redfish-core/include/redfish_aggregator.hpp
+++ b/redfish-core/include/redfish_aggregator.hpp
@@ -573,6 +573,35 @@ class RedfishAggregator
             return;
         }
 
+        if (aggType == AggregationType::Collection)
+        {
+            boost::urls::url& urlNew = localReq->url();
+            auto paramsIt = urlNew.params().begin();
+            while (paramsIt != urlNew.params().end())
+            {
+                const boost::urls::param& param = *paramsIt;
+                // only and $skip, params can't be passed to satellite
+                // as applying these filters twice results in different results.
+                // Removing them will cause them to only be processed in the
+                // aggregator. Note, this still doesn't work for collections
+                // that might return less than the complete collection by
+                // default, but hopefully those are rare/nonexistent in top
+                // collections.  bmcweb doesn't implement any of these.
+                if (param.key == "only" || param.key == "$skip")
+                {
+                    BMCWEB_LOG_DEBUG(
+                        "Erasing \"{}\" param from request to top level collection",
+                        param.key);
+
+                    paramsIt = urlNew.params().erase(paramsIt);
+                    continue;
+                }
+                // Pass all other parameters
+                paramsIt++;
+            }
+            localReq->target(urlNew.buffer());
+        }
+
         getSatelliteConfigs(
             std::bind_front(aggregateAndHandle, aggType, localReq, asyncResp));
     }