Skip to content

fix: normalize nil group param to prevent NoMethodError in version view - #1

Closed
makaiver with Copilot wants to merge 1 commit into
masterfrom
copilot/fixnil-group-param-nomethoderror
Closed

makaiver with Copilot wants to merge 1 commit into
masterfrom
copilot/fixnil-group-param-nomethoderror

Conversation

Copilot AI commented May 27, 2026

Copy link
Copy Markdown

Requesting /node/version/view?node=<node>&num=1 without group= can raise NoMethodError: undefined method '+' for nil from version.haml because the route passes params[:group] through as nil and the template concatenates it as a string. The same nil-path exists in /node/version/diffs; assigning groups to all devices avoids the crash, but only as a workaround.

  • Bug

    • params[:group] is nil when the query omits group=.
    • The version/diffs views assume @info[:group] is string-like and build group/node paths from it.
    • Repro:
      curl "http://<oxidized>:8888/node/version/view?node=<node>&num=1"
      # => HTTP 500
  • Change

    • Normalize params[:group] to '' in both route handlers:
      • GET /node/version/view
      • GET /node/version/diffs
    • This keeps the existing template guards working for “no group” requests without changing grouped behavior.
  • Code

    • Applied in webapp.rb:
      @info = {
        node: node,
        group: params[:group] || '',
        oid: params[:oid],
        time: Time.at(params[:epoch].to_i),
        num: params[:num]
      }
  • Coverage

    • Add focused route specs for requests that omit group= entirely, covering both version view and diffs paths.
    • Document the user-visible fix in CHANGELOG.md.
Original prompt

Open a pull request from the branch fix/nil-group-param-nomethoderror in makaiver/oxidized-web to ytti/oxidized-web:master.

The PR should:

  • Title: "fix: normalize nil group param to prevent NoMethodError in version view"
  • Description explaining:
    • The bug: requesting /node/version/view?node=<node>&num=1 without a group= parameter causes NoMethodError: undefined method '+' for nil in the version.haml template (line 5), because params[:group] returns nil and the template tries to concatenate it with a string.
    • The fix: normalize params[:group] to '' when absent using || '' in both the /node/version/view and /node/version/diffs route handlers in webapp.rb.
    • Reproduction: curl http://<oxidized>:8888/node/version/view?node=<node>&num=1 (no group param) → HTTP 500
    • No groups workaround: assigning groups to all devices avoids the bug but is not a proper fix.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
2026-05-27 07:58:09.981319 E [11:SemanticLogger::Appenders] SemanticLogger::Appenders -- Failed to log to appender: SemanticLogger::Appender::File -- Exception: ArgumentError: The supplied log file_name: /home/oxidized/.config/oxidized/logs is already a directory.
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:158:in reopen' /var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:185:in log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:32:in `block in log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:31:in `each'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:31:in `log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:151:in `process_messages'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:120:in `process'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:76:in `block in thread'
2026-05-27 07:58:09.982027 E [11:SemanticLogger::Appenders] SemanticLogger::Appenders -- Failed to log to appender: SemanticLogger::Appender::File -- Exception: ArgumentError: The supplied log file_name: /home/oxidized/.config/oxidized/logs is already a directory.
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:158:in `reopen'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:185:in `log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:32:in `block in log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:31:in `each'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:31:in `log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:151:in `process_messages'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:120:in `process'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:76:in `block in thread'
2026-05-27 07:58:09.983073 E [11:SemanticLogger::Appenders] SemanticLogger::Appenders -- Failed to log to appender: SemanticLogger::Appender::File -- Exception: ArgumentError: The supplied log file_name: /home/oxidized/.config/oxidized/logs is already a directory.
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:158:in `reopen'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:185:in `log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:32:in `block in log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:31:in `each'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:31:in `log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:151:in `process_messages'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:120:in `process'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/async.rb:76:in `block in thread'
2026-05-27 07:58:10.143687 E [11:SemanticLogger::Appenders] SemanticLogger::Appenders -- Failed to log to appender: SemanticLogger::Appender::File -- Exception: ArgumentError: The supplied log file_name: /home/oxidized/.config/oxidized/logs is already a directory.
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:158:in `reopen'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appender/file.rb:185:in `log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:32:in `block in log'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/lib/semantic_logger/appenders.rb:31:in `each'
/var/lib/gems/3.3.0/gems/semantic_logger-4.18.0/li...

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] fix: normalize nil group param to prevent NoMethodError in version view fix: normalize nil group param to prevent NoMethodError in version view May 27, 2026
Copilot AI requested a review from makaiver May 27, 2026 08:18
@makaiver makaiver closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants