[PR #14390] envconfig: return defaultValue on invalid boolean env var #45898

Open
opened 2026-04-25 01:30:31 -05:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/ollama/ollama/pull/14390
Author: @RajeshKumar11
Created: 2/24/2026
Status: 🔄 Open

Base: mainHead: fix/envconfig-bool-with-default


📝 Commits (1)

  • 2d5da7d envconfig: return defaultValue on invalid boolean env var

📊 Changes

2 files changed (+45 additions, -4 deletions)

View changed files

📝 envconfig/config.go (+2 -1)
📝 envconfig/config_test.go (+43 -3)

📄 Description

Problem

BoolWithDefault returns hardcoded true when strconv.ParseBool fails to parse the environment variable value, instead of returning defaultValue:

b, err := strconv.ParseBool(s)
if err != nil {
    return true  // bug: should be defaultValue
}

strconv.ParseBool only accepts "1", "0", "t", "f", "true", "false", "True", "False", "TRUE", "FALSE". Any other value — including the common "yes", "on", "enabled" — triggers the bug and silently enables boolean feature flags like OLLAMA_FLASH_ATTENTION, OLLAMA_NEW_ENGINE, etc.

Fixes #14389

Fix

  • return truereturn defaultValue so the caller's intended default is respected
  • Add slog.Warn to notify users of the misconfigured value, consistent with how Uint handles invalid input (see envconfig/config.go:238)

Tests

  • Updated TestBool to correct the expected values for invalid inputs ("random", "something") and added new cases for the most common invalid strings: "yes", "on", "enabled"
  • Added TestBoolWithDefault which explicitly tests both true and false defaults with invalid input values — something TestBool couldn't cover since Bool always uses false as the default

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/ollama/ollama/pull/14390 **Author:** [@RajeshKumar11](https://github.com/RajeshKumar11) **Created:** 2/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/envconfig-bool-with-default` --- ### 📝 Commits (1) - [`2d5da7d`](https://github.com/ollama/ollama/commit/2d5da7d014588bec49c7e5d145880aaf138ec311) envconfig: return defaultValue on invalid boolean env var ### 📊 Changes **2 files changed** (+45 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `envconfig/config.go` (+2 -1) 📝 `envconfig/config_test.go` (+43 -3) </details> ### 📄 Description ## Problem `BoolWithDefault` returns hardcoded `true` when `strconv.ParseBool` fails to parse the environment variable value, instead of returning `defaultValue`: ```go b, err := strconv.ParseBool(s) if err != nil { return true // bug: should be defaultValue } ``` `strconv.ParseBool` only accepts `"1"`, `"0"`, `"t"`, `"f"`, `"true"`, `"false"`, `"True"`, `"False"`, `"TRUE"`, `"FALSE"`. Any other value — including the common `"yes"`, `"on"`, `"enabled"` — triggers the bug and **silently enables** boolean feature flags like `OLLAMA_FLASH_ATTENTION`, `OLLAMA_NEW_ENGINE`, etc. Fixes #14389 ## Fix - `return true` → `return defaultValue` so the caller's intended default is respected - Add `slog.Warn` to notify users of the misconfigured value, consistent with how `Uint` handles invalid input (see `envconfig/config.go:238`) ## Tests - Updated `TestBool` to correct the expected values for invalid inputs (`"random"`, `"something"`) and added new cases for the most common invalid strings: `"yes"`, `"on"`, `"enabled"` - Added `TestBoolWithDefault` which explicitly tests both `true` and `false` defaults with invalid input values — something `TestBool` couldn't cover since `Bool` always uses `false` as the default --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-25 01:30:31 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/ollama#45898