Skip to content

Fix client certificate corruption by escaping plus signs before URL decoding#13871

Open
UmairStn wants to merge 5 commits into
wso2:masterfrom
UmairStn:fix/aws-alb-client-cert-decode
Open

Fix client certificate corruption by escaping plus signs before URL decoding#13871
UmairStn wants to merge 5 commits into
wso2:masterfrom
UmairStn:fix/aws-alb-client-cert-decode

Conversation

@UmairStn

Copy link
Copy Markdown

Problem
When client_certificate_encode = true, the gateway decodes the client certificate header via java.net.URLDecoder.decode(), which converts literal + characters into spaces.

AWS ALB forwards certificates using RFC 3986 rules, leaving + as a literal character. This causes URLDecoder to turn base64 + signs into spaces, corrupting the DER data and throwing java.io.IOException: Incomplete BER/DER data.

Solution
Replaced literal + characters with %2B right before the URLDecoder.decode() call. This ensures the decoder safely resolves them back to + instead of spaces, preserving the base64 structure.

@wso2-engineering wso2-engineering Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Agent Log Improvement Checklist

⚠️ Warning: AI-Generated Review Comments

  • The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
  • Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.

✅ Before merging this pull request:

  • Review all AI-generated comments for accuracy and relevance.
  • Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
Comment Accepted (Y/N) Reason
#### Log Improvement Suggestion No: 1
#### Log Improvement Suggestion No: 2

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@UmairStn, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 4 minutes and 13 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7552de25-c210-41c1-a102-ef2415527e62

📥 Commits

Reviewing files that changed from the base of the PR and between 741ebb8 and 342f726.

📒 Files selected for processing (1)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java
📝 Walkthrough

Walkthrough

Updates Utils.getClientCertificateFromHeader to replace literal '+' with '%2B' and perform UTF-8 URL-decoding only when the certificate string is non-null; decoding exceptions are wrapped in APIManagementException.

Changes

Client certificate handling

Layer / File(s) Summary
URL-decoding with plus-character escaping and null guard
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java
getClientCertificateFromHeader replaces literal + with %2B before URL-decoding the certificate string and adds a null-check to guard decoding; decoding errors throw APIManagementException.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: escaping plus signs before URL decoding to fix client certificate corruption.
Description check ✅ Passed The description clearly explains the problem (URLDecoder converting + to spaces), root cause (AWS ALB RFC 3986 forwarding), and solution (replacing + with %2B).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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

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

UmairStn and others added 2 commits June 11, 2026 19:03
…/org/wso2/carbon/apimgt/gateway/handlers/Utils.java

Co-authored-by: wso2-engineering[bot] <229087779+wso2-engineering[bot]@users.noreply.github.com>
…/org/wso2/carbon/apimgt/gateway/handlers/Utils.java

Co-authored-by: wso2-engineering[bot] <229087779+wso2-engineering[bot]@users.noreply.github.com>
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 11, 2026
@UmairStn

UmairStn commented Jun 11, 2026

Copy link
Copy Markdown
Author

AI Agent Log Improvement Checklist

⚠️ Warning: AI-Generated Review Comments

  • The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
  • Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.

✅ Before merging this pull request:

  • Review all AI-generated comments for accuracy and relevance.
  • Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.

Comment Accepted (Y/N) Reason
#### Log Improvement Suggestion No: 1 | Y | Added a debug log to trace client certificate processing. |
#### Log Improvement Suggestion No: 2 | Y | Added an error log inside the catch block to improve troubleshooting. |

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.

1 participant