Skip to content

fix: parse OPENAI_CUSTOM_HEADERS with commas inside quoted values#9306

Open
DOsinga wants to merge 1 commit into
mainfrom
fix/8495-shlex-custom-headers
Open

fix: parse OPENAI_CUSTOM_HEADERS with commas inside quoted values#9306
DOsinga wants to merge 1 commit into
mainfrom
fix/8495-shlex-custom-headers

Conversation

@DOsinga
Copy link
Copy Markdown
Collaborator

@DOsinga DOsinga commented May 18, 2026

Use shlex for quote-aware tokenization instead of naively splitting on commas. This allows header values to contain commas when quoted, e.g.:

OPENAI_CUSTOM_HEADERS='x-litellm-tags="tag1,tag2",x-other=plain'

The approach swaps commas↔spaces so shlex (which splits on whitespace) splits on the original commas while preserving commas inside quoted values. After tokenization, the swap is reversed.

Fixes #8495
Supersedes #9252

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af37923ed6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose/src/providers/openai.rs Outdated
Comment on lines +839 to +840
shlex::split(&swapped)
.unwrap_or_default()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid dropping all headers on shlex parse errors

Using shlex::split(...).unwrap_or_default() makes the whole header map silently empty when the input has shell-quote syntax errors, which now includes common raw header values containing an apostrophe (for example x-note=Bob's) because shlex treats ' as a quote delimiter. In that scenario, OPENAI_CUSTOM_HEADERS is effectively ignored and every custom header is lost, which is a regression from the previous comma-split behavior and can break requests that depend on these headers for routing/auth.

Useful? React with 👍 / 👎.

@DOsinga DOsinga force-pushed the fix/8495-shlex-custom-headers branch from af37923 to 23a9d04 Compare May 18, 2026 15:08
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23a9d04b90

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose/src/providers/openai.rs Outdated
Comment on lines +837 to +838
shlex::split(&swapped)
.unwrap_or_default()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve literal backslashes in custom header values

Switching to shlex::split makes OPENAI_CUSTOM_HEADERS parse with shell escape semantics, so unquoted backslashes are consumed instead of preserved. For example, x-path=C:\temp,x-other=ok now parses x-path as C:temp (backslash removed), whereas the previous comma-split logic kept the literal value. This silently corrupts header values that contain backslashes (paths, escaped identifiers, or signature-like values) before they are sent on requests.

Useful? React with 👍 / 👎.

Use shlex for quote-aware tokenization instead of naively splitting on
commas. This allows header values to contain commas when quoted, e.g.:
  OPENAI_CUSTOM_HEADERS='x-litellm-tags="tag1,tag2",x-other=plain'

Fixes #8495

Signed-off-by: Douwe Osinga <douwe@squareup.com>
@DOsinga DOsinga force-pushed the fix/8495-shlex-custom-headers branch from 23a9d04 to 144acc5 Compare May 18, 2026 15:39
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 144acc50ac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +837 to +838
match after_quote.split_once('"') {
Some((value, after_close)) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Parse escaped quotes inside quoted header values

The quoted-value branch terminates at the first " character via split_once('"'), even when that quote is escaped inside the value. For inputs like x-meta="{\"a\":1,\"b\":2}",x-id=1, this truncates x-meta to {\ and turns the remaining JSON into the next header key, which can corrupt headers or fail provider initialization when HeaderName::from_bytes runs. This breaks valid quoted header payloads (notably JSON metadata) that contain commas and embedded quotes.

Useful? React with 👍 / 👎.

let (value, remainder) = if let Some(after_quote) = after_eq.strip_prefix('"') {
match after_quote.split_once('"') {
Some((value, after_close)) => {
(value, after_close.strip_prefix(',').unwrap_or(after_close))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow whitespace before comma after quoted values

After closing a quoted value, the parser only strips a comma if it is the very next character. A header string such as x-tags="a,b" ,x-other=1 leaves ",x-other" attached to the next key path, producing a key like ,x-other and causing invalid-header-name errors later. This makes otherwise human-formatted inputs fail as soon as a quoted value is followed by space+comma instead of a bare comma.

Useful? React with 👍 / 👎.

@jamadeo
Copy link
Copy Markdown
Member

jamadeo commented May 22, 2026

shlex does seem like an awkward fit here.

What if we keep , as the separator and just allow escaping with \,, and \\ means literal backslash?

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.

Not respecting OPENAI_CUSTOM_HEADERS that have commas in values

2 participants