Skip to content

Corregir crash al parsear Subject DN con comas en la razón social #7

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
mendozagit merged 1 commit into from
May 5, 2026
Merged

Corregir crash al parsear Subject DN con comas en la razón social #7

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions Core/Certificate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ public string CertificateNumber
public List<KeyValuePair<string, string>> IssuerKeyValuePairs
{
get => _x509Certificate2.Issuer.Split(',')
.Select(x => new KeyValuePair<string, string>(x.Split('=')[0].Trim(), x.Split('=')[1].Trim())).ToList();
.Select(x => x.Split('=', 2))
.Where(parts => parts.Length == 2)
.Select(parts => new KeyValuePair<string, string>(parts[0].Trim(), parts[1].Trim()))
.ToList();
}

/// <summary>
Expand All @@ -151,7 +154,10 @@ public string Issuer
public List<KeyValuePair<string, string>> SubjectKeyValuePairs
{
get => _x509Certificate2.Subject.Split(',')
.Select(x => new KeyValuePair<string, string>(x.Split('=')[0].Trim(), x.Split('=')[1].Trim())).ToList();
.Select(x => x.Split('=', 2))
.Where(parts => parts.Length == 2)
.Select(parts => new KeyValuePair<string, string>(parts[0].Trim(), parts[1].Trim()))
.ToList();
Comment on lines 156 to +160
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

X500DistinguishedName Decode UseNewLines .NET 8 escape comma

💡 Result:

In .NET 8, X500DistinguishedName supports formatting/decoding options via X500DistinguishedNameFlags. To control comma/newline escaping: 1) UseNewLines (for decoding/formatting) - X500DistinguishedNameFlags.UseNewLines (value 256) is the flag that indicates the distinguished name uses the new line character. This is part of the X500DistinguishedNameFlags enum used with X500DistinguishedName’s decode/encode operations. 2) UseCommas vs UseNewLines are mutually exclusive - The flags doc states you should use only one of UseCommas, UseNewLines, and UseSemicolons. 3) “escape comma” in the input string (parsing) - The X500DistinguishedName(string ...) constructor parses a textual DN using the platform parsing routine (it’s backed by an X500 name parsing implementation). If you pass an input like ...OU=Docs, Adatum..., the parser may fail with “invalid X500 name ... delimiter/value” depending on the exact escaping/quoting the parser expects. - There isn’t a dedicated “escape comma in the input string” flag. The flags primarily affect how .NET formats/decodes the DN, not whether it accepts arbitrary escaped-comma syntax in the constructor input. Practical guidance - If your goal is to round-trip and avoid comma-delimiter ambiguities, prefer building the DN with X500DistinguishedNameBuilder (stronger than parsing a raw escaped string). - If you must parse the raw input, you may need to rewrite/quote the escaped comma according to what the parser accepts (e.g., converting the escaped comma form into a quoted form around the attribute value). Relevant API surfaces - X500DistinguishedName.Decode(X500DistinguishedNameFlags) decodes using the specified characteristics. - X500DistinguishedNameFlags includes UseNewLines and UseCommas and documents their intended usage constraints.

Citations:

🌐 Web query:

.NET X509Certificate2 Subject RFC 2253 escaped comma format

💡 Result:

In .NET, X509Certificate2.SubjectName gives you an X500DistinguishedName object (not a raw RFC 2253 string). The X500DistinguishedName API uses a “comma-delimited distinguished name” string format internally, and escaping rules for special characters follow the LDAP/RFC distinguished-name string escaping mechanism. For the specific case of an escaped comma in RFC 2253 (i.e., a comma that is part of the attribute value, not an RDN delimiter): - RFC 2253 says the comma character “,” must be escaped in attribute values by prefixing it with a backslash. - So the correct representation of a literal comma inside an attribute value is: , (backslash + comma). RFC 2253 escaping rule (comma in value): - The character to escape is one of: ",", "^", "", ... (including comma). If it is in that list, it is prefixed by a backslash. Otherwise it may be hex-escaped. Practical .NET guidance: 1) Prefer to avoid manual parsing/escaping of Subject strings. Use X509Certificate2.GetNameInfo(...) when you just need specific components, because it “should always be used instead of parsing the certificate”. 2) If you must round-trip/format the DN string yourself, use X500DistinguishedName and its Decode/Name/Format facilities rather than string operations. The X500DistinguishedNameFlags enum controls formatting (use commas/semicolons, etc.), but RFC 2253 escaping is still required for commas that belong to attribute values. Example (conceptual): - If the certificate subject has an attribute like CN=“R. Smith, O=Big Company” (comma inside CN value), RFC 2253 requires the comma be escaped in the string form as CN=R. Smith, O=Big Company (the comma after “Smith” is preceded by a backslash so it’s not treated as an RDN separator). Answer to your query wording (“.NET X509Certificate2 Subject RFC 2253 escaped comma format”): - The correct RFC 2253 escaped-comma syntax is: \, i.e. a literal “comma inside an attribute value” is represented as a backslash followed by a comma.

Citations:

🌐 Web query:

X500DistinguishedName.Decode UseNewLines vs UseCommas practical example

💡 Result:

Practical difference - X500DistinguishedName.Decode(flag) uses the flag to choose the separator used when rendering RDNs (relative distinguished names) into a string. - X500DistinguishedNameFlags.UseCommas renders the DN using comma+space separators. - X500DistinguishedNameFlags.UseNewLines renders the DN using the platform new-line sequence instead of comma separators. These flags are mutually exclusive with each other (Microsoft notes only one of UseCommas/UseNewLines/UseSemicolons should be specified). What you should do in code 1) Create an X500DistinguishedName from the certificate/ASN.1 value (so the DN parsing is already done). 2) Call Decode(...) with either UseCommas or UseNewLines depending on how you want to display it. Example (C#) using System; using System.Security.Cryptography.X509Certificates; class Demo { static void Main { // Example: use the subject DN from a certificate you already have. // Replace with your own X509Certificate2. X509Certificate2 cert = GetSomeCertificate; X500DistinguishedName dn = new X500DistinguishedName(cert.SubjectName.Name); string commas = dn.Decode(X500DistinguishedNameFlags.UseCommas); string newLines = dn.Decode(X500DistinguishedNameFlags.UseNewLines); Console.WriteLine("-- commas --"); Console.WriteLine(commas); Console.WriteLine("-- new lines --"); Console.WriteLine(newLines); } static X509Certificate2 GetSomeCertificate => throw new NotImplementedException; } Notes about “UseNewLines vs UseCommas” - UseCommas affects only the rendered layout (separator) of the decoded string. - UseNewLines affects only the rendered layout (separator) of the decoded string. Common gotcha (input parsing vs output formatting) - The DN string constructor X500DistinguishedName(string, flags) is for parsing/encoding a DN into ASN.1. Separators/escaping in the input have to be valid for what the underlying encoder expects. - Separate from Decode formatting: if your input DN contains commas inside attribute values, you may need proper quoting/escaping; otherwise you can get “invalid X500 name … delimiter” exceptions. If you tell me your exact DN example and your target .NET version (e.g., .NET 6/7/8), I can show the safest way to display it (Decode) and the safest way to ingest it (constructor vs X500DistinguishedNameBuilder).

Citations:

🏁 Script executed:

# Check if the Certificate.cs file exists and review the current implementation
fd -e cs -p 'Certificate.cs' | head -5

Repository: FiscalAPI/fiscalapi-credentials-net

Length of output: 119

🏁 Script executed:

# Read the Certificate.cs file to verify the implementation
cat -n Core/Certificate.cs | head -200

Repository: FiscalAPI/fiscalapi-credentials-net

Length of output: 7611

🏁 Script executed:

# Search for the exact context around lines 134-138 and 156-160
sed -n '80,170p' Core/Certificate.cs | cat -n

Repository: FiscalAPI/fiscalapi-credentials-net

Length of output: 3698

🏁 Script executed:

# Search for test files related to Certificate.cs to understand the PR context
fd -e cs -p 'Test.*Certificate|Certificate.*Test' | head -10

Repository: FiscalAPI/fiscalapi-credentials-net

Length of output: 61

🏁 Script executed:

# Check git history for recent changes to Certificate.cs
git log --oneline -10 -- Core/Certificate.cs 2>/dev/null || echo "Git not available"

Repository: FiscalAPI/fiscalapi-credentials-net

Length of output: 152

🏁 Script executed:

# Search for any mentions of "comma" or "SEMILLAS" in the repo (PR context)
rg -i "comma|semillas" --type cs | head -20

Repository: FiscalAPI/fiscalapi-credentials-net

Length of output: 61

Silent data truncation when Organization name contains commas: requires proper RFC 2253 parsing

The crash is fixed (no more IndexOutOfRangeException), but Split(',') on RFC 2253–formatted Subject DNs causes silent truncation when attribute values contain commas. In RFC 2253, a comma within a value is escaped as \, (e.g., O=SEMILLAS\, HORTALIZAS Y FLORES, C=MX). Naively splitting by , fragments this into O=SEMILLAS\, HORTALIZAS Y FLORES, C=MX—the middle fragment is silently dropped by the Where(parts => parts.Length == 2) filter, leaving Organization with a truncated and corrupted value (SEMILLAS\ instead of the full name). This is worse than the original crash because it silently corrupts the Organization field, which is critical for Mexican SAT invoicing.

Recommended fix: Use X500DistinguishedName.Decode(X500DistinguishedNameFlags.UseNewLines) to render each RDN on a separate line, preserving escaped characters within values. Extract a shared helper method to eliminate duplication (lines 134–138 and 156–160 are identical):

public List<KeyValuePair<string, string>> IssuerKeyValuePairs
{
    get => ParseDistinguishedName(_x509Certificate2.IssuerName);
}

public List<KeyValuePair<string, string>> SubjectKeyValuePairs
{
    get => ParseDistinguishedName(_x509Certificate2.SubjectName);
}

private static List<KeyValuePair<string, string>> ParseDistinguishedName(X500DistinguishedName dn)
{
    var formatted = dn.Decode(X500DistinguishedNameFlags.UseNewLines);
    return formatted
        .Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries)
        .Select(line => line.Split('=', 2))
        .Where(parts => parts.Length == 2)
        .Select(parts => new KeyValuePair<string, string>(parts[0].Trim(), parts[1].Trim()))
        .ToList();
}

Add a test with CN=FOO, O="ACME, S.A. DE C.V.", C=MX to verify Organization returns the full name with the embedded comma intact.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Core/Certificate.cs` around lines 156 - 160, The current
SubjectKeyValuePairs/IssuerKeyValuePairs logic naively splits Subject/Issuer
strings on commas which corrupts values containing escaped commas; replace both
identical blocks with a shared helper ParseDistinguishedName that accepts an
X500DistinguishedName, calls dn.Decode(X500DistinguishedNameFlags.UseNewLines),
splits on newline characters (not commas), then splits each line on '=' (limit
2), trims and returns List<KeyValuePair<string,string>>; update
SubjectKeyValuePairs and IssuerKeyValuePairs to call
ParseDistinguishedName(_x509Certificate2.SubjectName) /
ParseDistinguishedName(_x509Certificate2.IssuerName) and add a unit test with
CN=FOO, O="ACME, S.A. DE C.V.", C=MX to assert Organization preserves the
embedded comma.

}

/// <summary>
Expand Down