Skip to content

Fixes Issue 915#924

Merged
michelebastione merged 4 commits intomini-software:masterfrom
michelebastione:issue-915-fix
Feb 15, 2026
Merged

Fixes Issue 915#924
michelebastione merged 4 commits intomini-software:masterfrom
michelebastione:issue-915-fix

Conversation

@michelebastione
Copy link
Contributor

When trying to fill the cell of a template with a numeric value, there was a seemingly purposeless check that prevented it to be converted to a culturally invariant string if the data was fetched from a Dictionary or a DataTable. This PR removes the check and thus closes #915. A test and a relative template sample have also been added.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @michelebastione, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves an issue where numeric values originating from Dictionary or DataTable objects were not consistently formatted into culturally invariant strings when used in Excel templates. The fix ensures that all numeric data types are correctly represented in the generated Excel files, regardless of their source, by removing a specific bypass condition. A new test and sample template have been included to confirm the fix and prevent regressions.

Highlights

  • Bug Fix: Removed a conditional check that previously prevented numeric values sourced from Dictionary or DataTable from being converted to culturally invariant strings when filling templates.
  • Test Coverage: Introduced a new test case (TestIssue915) to specifically validate the correct handling and formatting of numeric values from dictionaries within templates.
  • Template Sample: Added a corresponding sample Excel template (TestIssue915.xlsx) to support the new test case.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs
    • Updated writer.WriteAsync calls to consistently use .AsMemory() with cancellationToken.
    • Refactored the GenerateCellValuesAsync method signature for improved readability by distributing parameters across multiple lines.
    • Modified the logic for retrieving cell values, introducing nullable reference types (object?, string?) and utilizing a C# switch expression for PropertyInfoOrFieldInfo.
    • Replaced CollectionExtensions.GetValueOrDefault with a direct call to replacements.GetValueOrDefault for .NET Core 3.0+ targets.
    • Applied null-forgiving operators (!) to dict and dataRow access where nullability was previously implicit.
    • Removed the !isDictOrTable condition from the numeric type check, enabling culturally invariant string conversion for numeric values from all data sources.
    • Ensured rowXml.Replace handles potentially null cellValueStr by providing an empty string fallback.
    • Removed redundant parentheses from the new GenerateCellValuesContext() instantiation.
  • tests/MiniExcel.OpenXml.Tests/MiniExcelIssueTests.cs
    • Added TestIssue915 to verify that numeric values from a dictionary are correctly applied and read from a generated Excel template, asserting their decimal values.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses issue #915 by fixing the formatting of numeric values from Dictionary or DataTable sources in templates. While the implementation includes modern C# syntax improvements and a new test case, a critical security vulnerability was identified: the template processing logic is susceptible to Formula Injection, as user-provided data starting with $= is automatically converted into Excel formulas without proper sanitization or escaping. Additionally, one critical code issue and one suggestion for improvement were noted.

}
}

replacements[key] = cellValueStr;
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

There is a potential ArgumentNullException here. cellValueStr is a nullable string (string?), but it's being assigned as a value to a Dictionary<string, string>, which does not accept null values. If ConvertToDateTimeString or another path results in a null cellValueStr, this line will throw an exception at runtime. You should use the null-coalescing operator to provide a default empty string, which seems to be the intent, as seen in the following line.

                    replacements[key] = cellValueStr ?? "";


replacements[key] = cellValueStr;
rowXml.Replace($"@header{{{{{key}}}}}", cellValueStr);
rowXml.Replace($"@header{{{{{key}}}}}", cellValueStr ?? "");
Copy link
Contributor

Choose a reason for hiding this comment

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

security-high high

The application is vulnerable to Formula Injection (CSV Injection). User-provided data is inserted into the Excel XML without sanitization against formula characters. Specifically, any string starting with $= is later processed by the ProcessFormulas method and converted into an executable Excel formula. An attacker can provide a malicious string (e.g., starting with $=HYPERLINK or $=WEBSERVICE) which will be executed when a user opens the generated Excel file. This can lead to data exfiltration or other malicious actions.

To remediate this, sanitize user-provided data before inserting it into the Excel XML. If a value starts with $=, it should be escaped (e.g., by prefixing it with a single quote ').

rowXml.Replace($"@header{{{{{key}}}}}", (cellValueStr?.StartsWith("$=") == true ? "&apos;" + cellValueStr : cellValueStr) ?? "");

var replacements = new Dictionary<string, string>();
#if NETCOREAPP3_0_OR_GREATER
string MatchDelegate(Match x) => CollectionExtensions.GetValueOrDefault(replacements, x.Groups[1].Value, "");
string MatchDelegate(Match x) => replacements.GetValueOrDefault(x.Groups[1].Value, "");
Copy link
Contributor

Choose a reason for hiding this comment

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

security-high high

This line retrieves user-provided values for template replacement. If these values start with $=, they will be interpreted as formulas in the resulting Excel file, leading to Formula Injection. See the comment on line 800 for more details and remediation steps.

Comment on lines +3806 to +3808
Assert.Equal(6, result[0].Altitude);
Assert.Equal(7.4, result[1].Altitude);
Assert.Equal(8.6, result[2].Altitude);
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For consistency and clarity in the test, it would be better to use floating-point literals for all assertions. The values read from Excel numeric cells are of type double. The current assertions mix an int literal (6) with double literals (7.4, 8.6), which is inconsistent. Using double literals for all assertions makes the expected type explicit.

        Assert.Equal(6.0, result[0].Altitude);
        Assert.Equal(7.4, result[1].Altitude);
        Assert.Equal(8.6, result[2].Altitude);

When trying to fill the cell of a template with a numeric value, there was a seemingly purposeless check that prevented the value to be converted to a culturally invariant string if the data was fetched from a Dictionary or a DataTable. This commit removes the check.
A test and a template sample have also been added.
In mini-software#917 sharedString element tags for templated data were changed from x:v to x:is but the formula placeholders did not take into account. This commit fixes that.
@michelebastione michelebastione merged commit 4f7f451 into mini-software:master Feb 15, 2026
3 checks passed
@michelebastione michelebastione deleted the issue-915-fix branch February 15, 2026 23:38
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.

[BUG] - [Numeric Value (decimal) in dictionary causes Excel erros]

1 participant