Skip to content

Fix GitHub API endpoint selection based on host parsing #1850

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix GitHub API endpoint selection based on host parsing #1850

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
52 changes: 50 additions & 2 deletions src/vcpkg/base/downloads.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,46 @@ namespace
cmd.string_arg("-H").string_arg(header);
}
}

// Extracts the host part from a URL string.
std::string extract_host(StringView url)
Copy link
Member

@BillyONeal BillyONeal Nov 24, 2025

Choose a reason for hiding this comment

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

This thing needs unit tests.

Copy link
Member

@BillyONeal BillyONeal Nov 24, 2025

Choose a reason for hiding this comment

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

URL parsing is an insanely complicated problem; a parsing function that just returns whether we get github.com or not may be easier. Otherwise we may want to look at using trurl.

Copy link
Author

@bc-lee bc-lee Nov 25, 2025

Choose a reason for hiding this comment

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

Thanks for the review. I’ll revisit this once the curl library integration has been re-landed.

{
// Remove scheme if present
StringView http_scheme = "http://";
StringView https_scheme = "https://";
if (url.starts_with(http_scheme))
{
url = url.substr(http_scheme.size());
}
else if (url.starts_with(https_scheme))
{
url = url.substr(https_scheme.size());
}

// Remove userinfo if present (e.g., user:pass@host)
const char* at_sign = Strings::find_first_of(url, "@");
Copy link
Member

@BillyONeal BillyONeal Nov 24, 2025

Choose a reason for hiding this comment

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

Seems little reason to use find_first_of when there is only one char you're looking for?

size_t at_pos = at_sign[0] == '\0' ? std::string::npos : static_cast<size_t>(at_sign - url.data());
Comment on lines +54 to +55
Copy link
Member

@BillyONeal BillyONeal Nov 24, 2025

Choose a reason for hiding this comment

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

This is undefined behavior: you can't dereference the returned iterator if there was no match. And there is no requirement that there is a \0 after a StringView

Copy link
Member

@BillyONeal BillyONeal Nov 24, 2025

Choose a reason for hiding this comment

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

Ditto several times below

if (at_pos != std::string::npos && at_pos + 1 < url.size())
{
// Move past the '@'
url = url.substr(at_pos + 1);
}

// Find the start of the path (first '/')
const char* first_slash = Strings::find_first_of(url, "/");
size_t slash_pos = first_slash[0] == '\0' ? std::string::npos : static_cast<size_t>(first_slash - url.data());
StringView host_part = slash_pos != std::string::npos ? url.substr(0, slash_pos) : url;

// Remove port if present (e.g., host:port)
const char* colon = Strings::find_first_of(host_part, ":");
size_t colon_pos = colon[0] == '\0' ? std::string::npos : static_cast<size_t>(colon - host_part.data());
if (colon_pos != std::string::npos)
{
host_part = host_part.substr(0, colon_pos);
}

return std::string(host_part);
}
}

namespace vcpkg
Expand Down Expand Up @@ -786,8 +826,16 @@ namespace vcpkg
std::string uri;
if (auto github_server_url = maybe_github_server_url.get())
{
uri = *github_server_url;
uri.append("/api/v3");
const auto host = extract_host(*github_server_url);
if (host != "github.com")
{
uri = *github_server_url;
uri.append("/api/v3");
}
else
{
uri = "https://api.github.com";
}
}
else
{
Expand Down
Loading