Skip to content

Conversation

jokemanfire
Copy link
Collaborator

The format method is not good for url.
use authorization_endpoint: format!("{}/authorize", auth_base) will cause the result
"http://example.com//authorize"
replace with url parse will be great for avoiding this problem.
fix #397

Motivation and Context

fix url bug.

How Has This Been Tested?

No need UT.

Breaking Changes

NO

Types of changes

  • Bug fix (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 change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@github-actions github-actions bot added T-core Core library changes T-transport Transport layer changes labels Aug 29, 2025
@jokemanfire jokemanfire requested review from Copilot and 4t145 August 29, 2025 12:46
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes URL parsing issues in the OAuth authentication module by replacing string formatting with proper URL parsing to prevent double slashes in endpoint URLs.

  • Introduces a helper function to properly construct endpoint URLs using URL parsing
  • Replaces hardcoded "http://localhost" strings with a named constant
  • Fixes the bug where format!("{}/authorize", auth_base) could produce URLs with double slashes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

The format method is not good for url.
@alexhancock alexhancock merged commit 048f1ac into modelcontextprotocol:main Aug 29, 2025
11 checks passed
@github-actions github-actions bot mentioned this pull request Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-core Core library changes T-transport Transport layer changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Double slashes in AuthorizationManager URLs
2 participants