Made Header Parameters for Security Schemes safe to use as C# variable name

[smoerijf]

Header names should only contain a-zA-Z0-9 and - (dash) characters according to the specification, but the dash is a problem for c# variable names.
The current ReplaceUnsafeCharacters lets the - (dash) pass through, which creates a problem for headers like "x-something-something".
ReplaceUnsafeCharacters initially was added to fix . (punctuation), this is still replaced by _ (underscore), ref: #653

Summary by CodeRabbit

• Bug Fixes

  • Consistently replace non‑alphanumeric characters with underscores in API header and parameter names to avoid unsafe/invalid identifiers.

• Other

  • Sample API specs updated to use safer header names for affected security schemes.

• Tests

  • Test resources and assertions updated to validate the revised header/parameter naming and resource encoding cleanup.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bbb0a330-a32f-4f0b-bd7f-6770140e3465

📥 Commits

Reviewing files that changed from the base of the PR and between ca61940 and 9f82940.

📒 Files selected for processing (1)

• src/Refitter.Tests/SwaggerPetstoreApizrTests.cs

---

📝 Walkthrough

Walkthrough

ReplaceUnsafeCharacters in ParameterExtractor now treats any non-letter-or-digit character as unsafe and replaces it with '_'. Test API specs and unit test assertions were updated to use auth.key-test_2 to validate the broader sanitization rule.

Changes

Cohort / File(s)	Summary
Core Logic src/Refitter.Core/ParameterExtractor.cs	Changed ReplaceUnsafeCharacters to replace any character that is not a Unicode letter or digit with '_' (previously targeted UnicodeCategory.OtherPunctuation).
Test API specs (V2 & V3) src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.json, src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml, src/Refitter.Tests/Resources/V3/SwaggerPetstoreWithUnsafeAuthenticationHeaders.json, src/Refitter.Tests/Resources/V3/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml	Updated security header name from auth.key to auth.key-test_2 (and removed UTF-8 BOM in one JSON) to exercise hyphen and dot handling in sanitization.
Tests (assertions) src/Refitter.Tests/SwaggerPetstoreTests.cs, src/Refitter.Tests/SwaggerPetstoreApizrTests.cs	Updated assertions to expect generated header name auth.key-test_2 and corresponding sanitized parameter identifier auth_key_test_2.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Made Security Header Parameters safe for C# when unsafe characters are present #663: Previously adjusted ReplaceUnsafeCharacters behavior for sanitizing security header-derived parameter names; closely related to the broadened replacement logic here.
• Fix PR #897 review feedback and add comprehensive bearer auth tests #936: Modifies the same ParameterExtractor.cs area for header/scheme handling; relevant at the file/class level.

Suggested labels

enhancement, .NET

Suggested reviewers

• christianhelle

Poem

🐰 I nibble at dots and dashes with glee,
Any non-letter or digit becomes underscore for me,
Specs now say auth.key-test_2 with pride,
Tests hop along, no unsafe chars to hide,
A little rabbit patch—clean code, happy stride. 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: making header parameters safe for use as C# variable names by replacing unsafe characters.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml (1)

554-558: ⚠️ Potential issue | 🟠 Major

Update securityDefinitions.auth_key.name to match V2 JSON file.

The V2 YAML file has name: "auth.key" in securityDefinitions (line 557), but the V2 JSON file correctly uses "name": "auth.key-test_2" (line 790). This inconsistency will cause the test Can_Generate_Code_With_Unsafe_AuthenticationHeaders to generate different header attributes when using the YAML vs JSON specification.

Proposed fix

 securityDefinitions:
   auth_key:
     type: "apiKey"
-    name: "auth.key"
+    name: "auth.key-test_2"
     in: "header"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml`
around lines 554 - 558, The YAML's securityDefinitions entry for auth_key has
the wrong header name; update securityDefinitions.auth_key.name from "auth.key"
to "auth.key-test_2" so it matches the V2 JSON and the expectations of the
Can_Generate_Code_With_Unsafe_AuthenticationHeaders test; locate the auth_key
block (securityDefinitions.auth_key) and change the name value to
"auth.key-test_2".

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml`:
- Around line 554-558: The YAML's securityDefinitions entry for auth_key has the
wrong header name; update securityDefinitions.auth_key.name from "auth.key" to
"auth.key-test_2" so it matches the V2 JSON and the expectations of the
Can_Generate_Code_With_Unsafe_AuthenticationHeaders test; locate the auth_key
block (securityDefinitions.auth_key) and change the name value to
"auth.key-test_2".

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: acc51ed5-f40a-47c8-b462-5c2d2a6adb4b

📥 Commits

Reviewing files that changed from the base of the PR and between 7c88426 and ca61940.

📒 Files selected for processing (6)

• src/Refitter.Core/ParameterExtractor.cs
• src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.json
• src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml
• src/Refitter.Tests/Resources/V3/SwaggerPetstoreWithUnsafeAuthenticationHeaders.json
• src/Refitter.Tests/Resources/V3/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml
• src/Refitter.Tests/SwaggerPetstoreTests.cs

[christianhelle]

@smoerijf good catch! thanks for taking the time to fix this

The core change looks good to me. Can you fix the tests? They seem to be failing

[christianhelle]

@all-contributors please add @smoerijf for bugs and code

[allcontributors]

@christianhelle

I've put up a pull request to add @smoerijf! 🎉

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

This PR updates Refitter’s header-parameter name sanitization so security-scheme header names containing non-alphanumeric characters (notably -) don’t produce invalid C# parameter identifiers, and updates the Petstore fixtures/tests to validate the new behavior.

Changes:

• Update ReplaceUnsafeCharacters to replace any non-letter/digit characters with _ when generating security-scheme header parameter names.
• Update Swagger Petstore test assertions to expect sanitized header parameter identifiers for unsafe header names.
• Update OpenAPI v2/v3 Petstore fixtures to use an unsafe header name containing . / - / _ for coverage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Refitter.Core/ParameterExtractor.cs	Broadens header-name sanitization for security-scheme header parameters.
src/Refitter.Tests/SwaggerPetstoreTests.cs	Updates assertion for generated header parameter name.
src/Refitter.Tests/SwaggerPetstoreApizrTests.cs	Updates assertion for generated header parameter name (Apizr).
src/Refitter.Tests/Resources/V3/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml	Updates v3 YAML fixture security scheme header name.
src/Refitter.Tests/Resources/V3/SwaggerPetstoreWithUnsafeAuthenticationHeaders.json	Updates v3 JSON fixture security scheme header name.
src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml	Updates v2 YAML fixture operation header parameter name (but currently inconsistent with securityDefinitions).
src/Refitter.Tests/Resources/V2/SwaggerPetstoreWithUnsafeAuthenticationHeaders.json	Updates v2 JSON fixture securityDefinitions header name (but currently inconsistent with an explicit operation header parameter).

Comments suppressed due to low confidence (1)

src/Refitter.Core/ParameterExtractor.cs:169

• ReplaceUnsafeCharacters still doesn’t guarantee a valid C# identifier (e.g., header names that start with a digit or match a reserved keyword would generate uncompilable parameter names). Since IdentifierUtils.ToCompilableIdentifier already exists and handles leading characters + reserved keywords, consider using it here (and removing this custom sanitizer) to truly make security-scheme header parameters “safe to use as C# variable name”.

    private static string ReplaceUnsafeCharacters(
        string unsafeText)
    {
        var safeText = new StringBuilder(unsafeText.Length);
        foreach (var character in unsafeText)
        {
            var safeCharacter = character;
            if (!char.IsLetterOrDigit(character))
            {
                safeCharacter = '_';
            }

            safeText.Append(safeCharacter);
        }

        return safeText.ToString();

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.15%. Comparing base (1a4d7e3) to head (9f82940).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #977   +/-   ##
=======================================
  Coverage   93.15%   93.15%           
=======================================
  Files          27       27           
  Lines        1797     1797           
=======================================
  Hits         1674     1674           
  Misses         49       49           
  Partials       74       74           

Flag	Coverage Δ
unittests	93.15% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christianhelle]

@smoerijf thank you for the contribution

[smoerijf]

@christianhelle Any chance to get a (preview) release from the main (with this fix)?

[christianhelle]

@christianhelle Any chance to get a (preview) release from the main (with this fix)?

@smoerijf A release preview is on its way

[christianhelle]

This fix is included in v1.8.0-preview.102