Skip to content

Fix string escaping, null safety, and numeric literals in optional parameter default values #804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
christianhelle merged 5 commits into from
Nov 11, 2025
Merged

Fix string escaping, null safety, and numeric literals in optional parameter default values #804

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
01f7e14
Initial plan
Copilot Nov 7, 2025
797b138
Address code review feedback for optional parameter default values
Copilot Nov 7, 2025
e17d732
Fix SonarCloud issues: optimize string escaping and fix null referenc…
Copilot Nov 9, 2025
7402111
Add comprehensive test coverage for string escaping and numeric liter…
Copilot Nov 9, 2025
c7ca8f8
Apply code review feedback: improve string escaping and numeric liter…
Copilot Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 51 additions & 5 deletions src/Refitter.Core/ParameterExtractor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,11 @@ private static string GetDefaultValueForParameter(string parameterString, IColle
var variableName = parts[parts.Length - 1].TrimEnd(';', ',');

var parameterModel = parameterModels.FirstOrDefault(p => p.VariableName == variableName);
return parameterModel?.Schema?.Default != null
? FormatDefaultValue(parameterModel.Schema.Default, parameterModel.Type)
: "default";
if (parameterModel?.Schema?.Default != null && !string.IsNullOrEmpty(parameterModel.Type))
{
return FormatDefaultValue(parameterModel.Schema.Default, parameterModel.Type);
}
return "default";
}

private static string FormatDefaultValue(object? defaultValue, string parameterType)
Expand All @@ -160,12 +162,56 @@ private static string FormatDefaultValue(object? defaultValue, string parameterT
return type switch
{
"bool" => defaultValue.ToString()?.ToLowerInvariant() ?? "default",
"string" => $"\"{defaultValue}\"",
_ when IsNumericType(type) => defaultValue.ToString() ?? "default",
"string" => $"\"{EscapeString(defaultValue.ToString() ?? string.Empty)}\"",
_ when IsNumericType(type) => FormatNumericValue(defaultValue, type),
_ => "default"
};
}

private static string EscapeString(string value)
{
var sb = new StringBuilder(value.Length * 2);
Comment thread
christianhelle marked this conversation as resolved.
Outdated
foreach (var c in value)
{
switch (c)
{
case '\\':
sb.Append("\\\\");
break;
case '"':
sb.Append("\\\"");
break;
case '\n':
sb.Append("\\n");
break;
case '\r':
sb.Append("\\r");
break;
case '\t':
sb.Append("\\t");
break;
default:
sb.Append(c);
break;
}
}
return sb.ToString();
}
Comment on lines +171 to +211

Copilot AI Nov 9, 2025

Copy link

Choose a reason for hiding this comment

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

The EscapeString method should handle additional escape sequences for completeness. Consider adding cases for:

  • Form feed (\f → \\f)
  • Vertical tab (\v → \\v)
  • Backspace (\b → \\b)

While these are less common in API parameters, handling them ensures robustness when dealing with various string defaults.

Copilot uses AI. Check for mistakes.
Comment on lines +171 to +211

Copilot AI Nov 9, 2025

Copy link

Choose a reason for hiding this comment

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

The EscapeString method does not handle the null character (\0) which can appear in string defaults. If a string contains \0, it should be escaped as \\0 to be valid in C# string literals. Consider adding a case for '\0' that appends "\\0".

Copilot uses AI. Check for mistakes.

private static string FormatNumericValue(object defaultValue, string type)
{
var numericString = defaultValue is IFormattable formattable
? formattable.ToString(null, CultureInfo.InvariantCulture)
: (defaultValue.ToString() ?? "default");

return type switch
{
"float" or "Single" => $"{numericString}f",
"decimal" or "Decimal" => $"{numericString}m",

Copilot AI Nov 9, 2025

Copy link

Choose a reason for hiding this comment

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

The FormatNumericValue method doesn't handle long and ulong type suffixes. In C#, large integer values that exceed int.MaxValue (2,147,483,647) must have an L suffix to be treated as long literals. Without the suffix, values like long? value = 3000000000 will cause a compilation error. Add cases for:

  • "long" or "Int64" → append L
  • "ulong" or "UInt64" → append UL

Additionally, consider appending U for uint types to be explicit, though this is optional for values within int range.

Suggested change
"decimal" or "Decimal" => $"{numericString}m",
"decimal" or "Decimal" => $"{numericString}m",
"long" or "Int64" => $"{numericString}L",
"ulong" or "UInt64" => $"{numericString}UL",
"uint" or "UInt32" => $"{numericString}U",

Copilot uses AI. Check for mistakes.
_ => numericString
};
}

Comment on lines +222 to +230

Copilot AI Nov 9, 2025

Copy link

Choose a reason for hiding this comment

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

The FormatNumericValue method doesn't handle the double type suffix. While doubles don't require a suffix in most cases, if the numeric value is an integer (e.g., 1, 10), it will be treated as int by default in C# unless explicitly cast. For double types with integer values, you should append .0 to ensure the literal is treated as a double. For example, double? value = 10 should be double? value = 10.0 or double? value = 10d.

Suggested change
"decimal" or "Decimal" => $"{numericString}m",
_ => numericString
};
}
"decimal" or "Decimal" => $"{numericString}m",
"double" or "Double" => FormatDoubleLiteral(numericString),
_ => numericString
};
}
// Ensures that double literals are formatted with .0 if integer, or as-is if already floating-point
private static string FormatDoubleLiteral(string numericString)
{
// If the string already contains a decimal point or exponent, return as-is
if (numericString.Contains('.') || numericString.Contains('e') || numericString.Contains('E'))
return numericString;
// Otherwise, append .0 to make it a double literal
return numericString + ".0";
}

Copilot uses AI. Check for mistakes.
private static bool IsNumericType(string type)
{
return type is "int" or "Int32" or "long" or "Int64" or "short" or "Int16"
Expand Down
268 changes: 268 additions & 0 deletions src/Refitter.Tests/Examples/OptionalParametersWithDefaultValuesEdgeCasesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,268 @@
using FluentAssertions;
using Refitter.Core;
using Refitter.Tests.Build;
using Refitter.Tests.TestUtilities;
using Xunit;

namespace Refitter.Tests.Examples;

public class OptionalParametersWithDefaultValuesEdgeCasesTests
{
[Fact]
public async Task String_Default_Values_Should_Be_Escaped()
{
const string openApiSpec =
"""
{
"openapi": "3.0.1",
"info": {
"title": "Test API",
"version": "v1"
},
"paths": {
"/api/test": {
"get": {
"operationId": "TestEscaping",
"parameters": [
{
"name": "quotedString",
"in": "query",
"required": false,
"schema": {
"type": "string",
"default": "value with \"quotes\""
}
},
{
"name": "backslashString",
"in": "query",
"required": false,
"schema": {
"type": "string",
"default": "path\\to\\file"
}
},
{
"name": "newlineString",
"in": "query",
"required": false,
"schema": {
"type": "string",
"default": "line1\nline2"
}
}
],
"responses": {
"200": {
"description": "Success"
}
}
}
}
}
}
""";

string generatedCode = await GenerateCode(openApiSpec);

// Verify strings are properly escaped
generatedCode.Should().Contain("string? quotedString = \"value with \\\"quotes\\\"\"");
generatedCode.Should().Contain("string? backslashString = \"path\\\\to\\\\file\"");
generatedCode.Should().Contain("string? newlineString = \"line1\\nline2\"");
}

[Fact]
public async Task Float_Default_Values_Should_Have_Type_Suffix()
{
const string openApiSpec =
"""
{
"openapi": "3.0.1",
"info": {
"title": "Test API",
"version": "v1"
},
"paths": {
"/api/test": {
"get": {
"operationId": "TestFloat",
"parameters": [
{
"name": "floatValue",
"in": "query",
"required": false,
"schema": {
"type": "number",
"format": "float",
"default": 1.5
}
}
],
"responses": {
"200": {
"description": "Success"
}
}
}
}
}
}
""";

string generatedCode = await GenerateCode(openApiSpec);

// Verify float has 'f' suffix
generatedCode.Should().Contain("float? floatValue = 1.5f");
}

[Fact]
public async Task Decimal_Default_Values_Should_Have_Type_Suffix()
{
const string openApiSpec =
"""
{
"openapi": "3.0.1",
"info": {
"title": "Test API",
"version": "v1"
},
"paths": {
"/api/test": {
"get": {
"operationId": "TestDecimal",
"parameters": [
{
"name": "decimalValue",
"in": "query",
"required": false,
"schema": {
"type": "number",
"format": "decimal",
"default": 99.99
}
}
],
"responses": {
"200": {
"description": "Success"
}
}
}
}
}
}
""";

string generatedCode = await GenerateCode(openApiSpec);

// Verify decimal has 'm' suffix
generatedCode.Should().Contain("decimal? decimalValue = 99.99m");
}

[Fact]
public async Task Generated_Code_With_Escaped_Strings_Should_Build()
{
const string openApiSpec =
"""
{
"openapi": "3.0.1",
"info": {
"title": "Test API",
"version": "v1"
},
"paths": {
"/api/test": {
"get": {
"operationId": "TestBuild",
"parameters": [
{
"name": "escapedString",
"in": "query",
"required": false,
"schema": {
"type": "string",
"default": "value with \"quotes\" and \\backslashes\\"
}
}
],
"responses": {
"200": {
"description": "Success"
}
}
}
}
}
}
""";

string generatedCode = await GenerateCode(openApiSpec);
BuildHelper.BuildCSharp(generatedCode).Should().BeTrue();
}

[Fact]
public async Task Generated_Code_With_Float_Decimal_Should_Build()
{
const string openApiSpec =
"""
{
"openapi": "3.0.1",
"info": {
"title": "Test API",
"version": "v1"
},
"paths": {
"/api/test": {
"get": {
"operationId": "TestBuild",
"parameters": [
{
"name": "floatValue",
"in": "query",
"required": false,
"schema": {
"type": "number",
"format": "float",
"default": 1.5
}
},
{
"name": "decimalValue",
"in": "query",
"required": false,
"schema": {
"type": "number",
"format": "decimal",
"default": 99.99
}
}
],
"responses": {
"200": {
"description": "Success"
}
}
}
}
}
}
""";

string generatedCode = await GenerateCode(openApiSpec);
BuildHelper.BuildCSharp(generatedCode).Should().BeTrue();
}

private static async Task<string> GenerateCode(string openApiSpec)
{
var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(openApiSpec);
var settings = new RefitGeneratorSettings
{
OpenApiPath = swaggerFile,
OptionalParameters = true
};

var sut = await RefitGenerator.CreateAsync(settings);
var code = sut.Generate();
return code;
}
}
Loading