fix: normalize nil group param to prevent NoMethodError

CHANGELOG.md

@@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 ### Changed
 
 ### Fixed
+- Fix `NoMethodError` when `group=` param is absent from `/node/version/view` and `/node/version/diffs` requests. `params[:group]` was assigned directly to `@info[:group]`; the HAML templates and `get_diff` calls then called methods on `nil`. Normalise to `''` on input and pass `nil` to the oxidized backend where required (@makaiver)
+- Fix `NoMethodError` and `RegexpError` crashes on `/node/version` and `/nodes/conf_search` when params are missing or contain a malformed regex; return empty results instead of a 500 (@makaiver)
 
 
 ## [0.18.1 – 2026-01-19]

lib/oxidized/web/webapp.rb

@@ -65,13 +65,21 @@ class WebApp < Sinatra::Base
       end
 
       post '/nodes/conf_search.?:format?' do
-        @to_research = Regexp.new params[:search_in_conf_textbox]
-        nodes_list = nodes.list.map
+        search_term = params[:search_in_conf_textbox].to_s
         @nodes_match = []
-        nodes_list.each do |n|
-          node, @json = route_parse n[:name]
-          config = nodes.fetch node, n[:group]
-          @nodes_match.push({ node: n[:name], full_name: n[:full_name] }) if config[@to_research]
+        unless search_term.empty?
+          begin
+            @to_research = Regexp.new search_term
+          rescue RegexpError
+            @to_research = nil
+          end
+          if @to_research
+            nodes.list.map.each do |n|
+              node, @json = route_parse n[:name]
+              config = nodes.fetch node, n[:group]
+              @nodes_match.push({ node: n[:name], full_name: n[:full_name] }) if config[@to_research]
+            end
+          end
         end
         @data = @nodes_match
         out :conf_search
@@ -142,7 +150,7 @@ class WebApp < Sinatra::Base
         @data = nil
         @group = nil
         @node = nil
-        node_full = params[:node_full]
+        node_full = params[:node_full].to_s
         if node_full.include? '/'
           node_full = node_full.rpartition("/")
           @group = node_full[0]
@@ -160,13 +168,13 @@ class WebApp < Sinatra::Base
         node, @json = route_parse :node
         @info = {
           node: node,
-          group: params[:group],
+          group: params[:group] || '',
           oid: params[:oid],
           time: Time.at(params[:epoch].to_i),
           num: params[:num]
         }
 
-        the_data = nodes.get_version node, @info[:group], @info[:oid]
+        the_data = nodes.get_version node, @info[:group].empty? ? nil : @info[:group], @info[:oid]
         if %w[json text].include?(params[:format])
           @data = the_data
         else
@@ -181,7 +189,7 @@ class WebApp < Sinatra::Base
         node, @json = route_parse :node
         @data = nil
         @info = { node: node,
-                  group: params[:group],
+                  group: params[:group] || '',
                   oid: params[:oid],
                   time: Time.at(params[:epoch].to_i),
                   num: params[:num],
@@ -201,9 +209,9 @@ class WebApp < Sinatra::Base
             @info[:num2] = num
             break
           end
-          @data = nodes.get_diff node, @info[:group], @info[:oid], oid2
+          @data = nodes.get_diff node, group, @info[:oid], oid2
         else
-          @data = nodes.get_diff node, @info[:group], @info[:oid], nil
+          @data = nodes.get_diff node, group, @info[:oid], nil
         end
         @stat = %w[null null]
         if @data != 'no diffs' && !@data.nil?
@@ -254,7 +262,7 @@ def route_parse(param)
         e = if param.respond_to?(:to_str)
               param.split '.'
             else
-              params[param].split '.'
+              params[param].to_s.split '.'
             end
         if e.last == 'json'
           e.pop

spec/web/conf_search_spec.rb

@@ -0,0 +1,40 @@
+require_relative '../spec_helper'
+
+describe Oxidized::API::WebApp do
+  include Rack::Test::Methods
+
+  def app
+    Oxidized::API::WebApp
+  end
+
+  before do
+    @nodes = mock('Oxidized::Nodes')
+    app.set(:nodes, @nodes)
+  end
+
+  describe 'post /nodes/conf_search' do
+    it 'returns 200 with empty results when search_in_conf_textbox is missing' do
+      post '/nodes/conf_search'
+      _(last_response.ok?).must_equal true
+    end
+
+    it 'returns 200 with empty results when search_in_conf_textbox is blank' do
+      post '/nodes/conf_search', search_in_conf_textbox: ''
+      _(last_response.ok?).must_equal true
+    end
+
+    it 'returns 200 with empty results when regex is malformed (no crash)' do
+      post '/nodes/conf_search', search_in_conf_textbox: '['
+      _(last_response.ok?).must_equal true
+    end
+
+    it 'returns 200 with matches when regex is valid' do
+      @nodes.expects(:list).returns(
+        [{ name: 'sw5', full_name: 'sw5', group: nil }]
+      )
+      @nodes.expects(:fetch).with('sw5', nil).returns("hostname sw5\n")
+      post '/nodes/conf_search', search_in_conf_textbox: 'hostname'
+      _(last_response.ok?).must_equal true
+    end
+  end
+end

spec/web/node/version_spec.rb

@@ -71,11 +71,18 @@ def app
           "2025-02-05 19:49:00 +0100</td>\n"
         )).must_equal true
     end
+
+    it 'returns 200 when node_full is absent (no NoMethodError)' do
+      @nodes.expects(:version).with('', nil).returns([])
+
+      get '/node/version'
+      _(last_response.ok?).must_equal true
+    end
   end
 
   describe 'get /node/version/view.?:format?' do
     it 'fetches a previous version from git' do
-      @nodes.expects(:get_version).with('sw5', '', 'c8aa93cab5').returns('Old configuration of sw5')
+      @nodes.expects(:get_version).with('sw5', nil, 'c8aa93cab5').returns('Old configuration of sw5')
 
       get '/node/version/view?node=sw5&group=&oid=c8aa93cab5&epoch=1738781340&num=2'
       _(last_response.ok?).must_equal true
@@ -90,7 +97,7 @@ def app
     end
 
     it 'does not display binary content' do
-      @nodes.expects(:get_version).with('sw5', '', 'c8aa93cab5').returns("\xff\x42 binary content\x00")
+      @nodes.expects(:get_version).with('sw5', nil, 'c8aa93cab5').returns("\xff\x42 binary content\x00")
 
       get '/node/version/view?node=sw5&group=&oid=c8aa93cab5&epoch=1738781340&num=2'
       _(last_response.ok?).must_equal true
@@ -107,7 +114,7 @@ def app
 
     it 'does not encode html-chars in text-format' do
       configuration = "text &/<> \n ascii;"
-      @nodes.expects(:get_version).with('sw5', '', 'c8aa93cab5').returns(configuration)
+      @nodes.expects(:get_version).with('sw5', nil, 'c8aa93cab5').returns(configuration)
       get '/node/version/view?node=sw5&group=&oid=c8aa93cab5&epoch=1738781340&num=2&format=text'
 
       _(last_response.ok?).must_equal true
@@ -116,12 +123,20 @@ def app
 
     it 'does not encode html-chars in json-format' do
       configuration = "text &/<> \n ascii;"
-      @nodes.expects(:get_version).with('sw5', '', 'c8aa93cab5').returns(configuration)
+      @nodes.expects(:get_version).with('sw5', nil, 'c8aa93cab5').returns(configuration)
       get '/node/version/view?node=sw5&group=&oid=c8aa93cab5&epoch=1738781340&num=2&format=json'
 
       _(last_response.ok?).must_equal true
       _(last_response.body).must_equal '["text &/<> \n"," ascii;"]'
     end
+
+    it 'returns 200 when group param is absent (no NoMethodError)' do
+      @nodes.expects(:get_version).with('sw5', nil, 'c8aa93cab5').returns('Old configuration of sw5')
+
+      get '/node/version/view?node=sw5&oid=c8aa93cab5&epoch=1738781340&num=2'
+      _(last_response.ok?).must_equal true
+      _(last_response.body.include?('Old configuration of sw5')).must_equal true
+    end
   end
 
   describe 'get /node/version/diffs' do
@@ -157,5 +172,35 @@ def app
           "#{Time.at(1738781340)}</span>"
         )).must_equal true
     end
+
+    it 'returns 200 when group param is absent (no NoMethodError)' do
+      @versions = [
+        { oid: "C006", time: Time.parse("2025-02-05 19:49:00 +0100") },
+        { oid: "C003", time: Time.parse("2025-02-05 19:03:00 +0100") }
+      ]
+      @diff = { patch: "diff --git a/sw5 b/sw5\nsome diff\n", stat: [1, 1] }
+
+      @nodes.expects(:version).with('sw5', nil).returns(@versions)
+      @nodes.expects(:get_diff).with('sw5', nil, 'C006', nil).returns(@diff)
+
+      get '/node/version/diffs?node=sw5&oid=C006&epoch=1738781340&num=2'
+      _(last_response.ok?).must_equal true
+    end
+
+    it 'passes nil group to get_diff for ungrouped nodes' do
+      @versions = [
+        { oid: "C006", time: Time.parse("2025-02-05 19:49:00 +0100") },
+        { oid: "C003", time: Time.parse("2025-02-05 19:03:00 +0100") }
+      ]
+      @diff = { patch: "diff --git a/sw5 b/sw5\nsome diff\n", stat: [1, 1] }
+
+      # Explicitly assert nil is passed, not '' — empty string would resolve
+      # to wrong git path in oxidized's yield_repo_and_path
+      @nodes.expects(:version).with('sw5', nil).returns(@versions)
+      @nodes.expects(:get_diff).with('sw5', nil, 'C006', nil).returns(@diff)
+
+      get '/node/version/diffs?node=sw5&oid=C006&epoch=1738781340&num=2'
+      _(last_response.ok?).must_equal true
+    end
   end
 end