Skip to content

Conversation

@alexeyzimarev
Copy link
Member

@alexeyzimarev alexeyzimarev commented Jan 25, 2026

User description

Description

Fixes #2207

Purpose

This pull request is a:

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

PR Type

Bug fix


Description

  • Fix pipe character encoding when AddQueryParameter encode=false

  • Refactor URI building to preserve unencoded query parameters

  • Use UriBuilder instead of Uri constructor to prevent re-encoding

  • Add BuildUriString method returning string URI for proper character preservation


Diagram Walkthrough

flowchart LR
  A["AddQueryParameter<br/>encode=false"] --> B["BuildUriString<br/>returns string"]
  B --> C["UriBuilder preserves<br/>unencoded chars"]
  C --> D["HttpRequestMessage<br/>receives raw URL"]
  D --> E["Pipe char stays as |<br/>not %7C"]
Loading

File Walkthrough

Relevant files
Bug fix
BuildUriExtensions.cs
Add BuildUriString method for unencoded query preservation

src/RestSharp/BuildUriExtensions.cs

  • Refactored BuildUri to delegate to new BuildUriString method
  • Added BuildUriString method that returns string URI instead of Uri
    object
  • Manually concatenates query string to preserve unencoded characters
  • Avoids Uri constructor re-encoding when encode=false is specified
+15/-10 
UriExtensions.cs
Use UriBuilder to prevent query string re-encoding             

src/RestSharp/Request/UriExtensions.cs

  • Modified AddQueryString to use UriBuilder instead of Uri constructor
  • UriBuilder.Query property preserves unencoded characters
  • Properly handles existing query parameters with ampersand separator
+3/-3     
RestClient.Async.cs
Pass raw URL string to HttpRequestMessage                               

src/RestSharp/RestClient.Async.cs

  • Changed to use BuildUriString returning string instead of Uri
  • Pass raw URL string directly to HttpRequestMessage constructor
  • Preserves unencoded characters in the actual HTTP request
+3/-2     
Tests
DefaultParameterTests.cs
Add integration test for pipe character encoding                 

test/RestSharp.Tests.Integrated/DefaultParameterTests.cs

  • Added integration test for pipe character encoding with encode=false
  • Verifies that pipe character remains unencoded in actual HTTP request
  • Uses RequestBodyCapturer.RawUrl to check transmitted query string
+13/-0   
RequestBodyCapturer.cs
Add RawUrl property for URL capture                                           

test/RestSharp.Tests.Shared/Fixtures/RequestBodyCapturer.cs

  • Added RawUrl property to capture the actual URL string sent over HTTP
  • Stores raw URL before Uri object creation to preserve unencoded
    characters
+2/-0     
UrlBuilderTests.Get.cs
Add unit test for pipe character encoding                               

test/RestSharp.Tests/UrlBuilderTests.Get.cs

  • Added unit test for pipe character in query parameter with
    encode=false
  • Tests BuildUriString method to verify pipe character remains unencoded
  • Validates expected output format with pipe character preserved
+12/-0   
Miscellaneous
.DS_Store
macOS system file update                                                                 

.DS_Store

  • macOS system file with binary metadata changes
[link]   
.DS_Store
macOS system file update                                                                 

test/.DS_Store

  • macOS system file with binary metadata changes
[link]   

…en encode=false

The issue was that when AddQueryParameter was called with encode=false,
the pipe character (|) was still being encoded to %7C. This happened
because the AddQueryString method was creating a new Uri object directly,
and the Uri constructor automatically encodes certain characters including
the pipe character.

The fix uses UriBuilder instead, which preserves the query string as-is
without re-encoding it. This ensures that when encode=false is specified,
characters like pipe (|) remain unencoded as expected.

Added test case to verify pipe character is not encoded when encode=false.
…en encode=false

- Modified BuildUriExtensions to add BuildUriString method that returns a string URI
- Updated RestClient.Async to use string URI for HttpRequestMessage to preserve unencoded characters
- Added RawUrl property to RequestBodyCapturer to capture the actual URL string sent over HTTP
- Added integration test to verify pipe character is not encoded when encode=false
- All existing tests pass
@qodo-free-for-open-source-projects
Copy link
Contributor

qodo-free-for-open-source-projects bot commented Jan 25, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟢
🎫 #2207
🟢 Pipe character (|) should not be encoded when AddQueryParameter is called with
encode=false
Query parameter 'ids' with value 'in:001|116' should be transmitted as 'ids=in:001|116'
not 'ids=in:001%7C116'
The encode=false option should prevent encoding of the pipe character in query parameters
Codebase Duplication Compliance
🟢
No codebase code duplication found No new components were introduced in the PR code
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null validation: The BuildUriString method does not validate if mergedUri is null before accessing
AbsoluteUri property, which could lead to NullReferenceException.

Referred Code
var mergedUri = client.BuildUriWithoutQueryParameters(request);
var query     = client.GetRequestQuery(request);

if (query == null) return mergedUri.AbsoluteUri;

var separator = mergedUri.AbsoluteUri.Contains('?') ? "&" : "?";
return $"{mergedUri.AbsoluteUri}{separator}{query}";

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 25, 2026

Deploying restsharp with  Cloudflare Pages  Cloudflare Pages

Latest commit: 31f8508
Status: ✅  Deploy successful!
Preview URL: https://6a9299c9.restsharp.pages.dev
Branch Preview URL: https://fix-issue-2207-pipe-encoding.restsharp.pages.dev

View logs

@qodo-free-for-open-source-projects
Copy link
Contributor

qodo-free-for-open-source-projects bot commented Jan 25, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Remove unused local variable initialization

Remove the unused local variable url as it is initialized but never used.

src/RestSharp/RestClient.Async.cs [113-117]

 var httpMethod = AsHttpMethod(request.Method);
 var urlString  = this.BuildUriString(request);
-var url        = new Uri(urlString);
 
 using var message = new HttpRequestMessage(httpMethod, urlString);

[Suggestion processed]

Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies that the url variable is unused and can be removed, which is a valid code quality improvement.

Low
  • Update

@github-actions
Copy link

github-actions bot commented Jan 25, 2026

Test Results

   42 files     42 suites   20m 8s ⏱️
  496 tests   496 ✅ 0 💤 0 ❌
3 467 runs  3 467 ✅ 0 💤 0 ❌

Results for commit 31f8508.

♻️ This comment has been updated with latest results.

@alexeyzimarev alexeyzimarev force-pushed the fix-issue-2207-pipe-encoding branch from 5473f07 to 1fb6203 Compare January 26, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AddQueryParameter : Pipe character still encoded with false option

2 participants