Skip to content
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.

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

URL: avoid double slash at the start of the path #5187

Merged
merged 6 commits into from
Nov 28, 2023

Conversation

antkmsft
Copy link
Member

@antkmsft antkmsft commented Nov 25, 2023

Closes #5130.

Previous behavior:

  TEST(URL, LeadingSlashInPath)
  {
    Core::Url const u0("https://www.microsoft.com");
    Core::Url const u1("https://www.microsoft.com/");

    EXPECT_EQ(u0.GetAbsoluteUrl(), "https://www.microsoft.com");
    EXPECT_EQ(u1.GetAbsoluteUrl(), "https://www.microsoft.com");

    {
      auto url0 = u0;
      auto url1 = u1;
      url0.AppendPath("path");
      url1.AppendPath("path");
      EXPECT_EQ(url0.GetAbsoluteUrl(), "https://www.microsoft.com/path");
      EXPECT_EQ(url1.GetAbsoluteUrl(), "https://www.microsoft.com/path");
    }

    {
      auto url0 = u0;
      auto url1 = u1;
      url0.AppendPath("/path");
      url1.AppendPath("/path");
      EXPECT_EQ(url0.GetAbsoluteUrl(), "https://www.microsoft.com//path"); // <-- fixed in this PR
      EXPECT_EQ(url1.GetAbsoluteUrl(), "https://www.microsoft.com//path"); // <-- fixed in this PR
    }

    {
      auto url0 = u0;
      auto url1 = u1;
      url0.SetPath("path");
      url1.SetPath("path");
      EXPECT_EQ(url0.GetAbsoluteUrl(), "https://www.microsoft.com/path");
      EXPECT_EQ(url1.GetAbsoluteUrl(), "https://www.microsoft.com/path");
    }

    {
      auto url0 = u0;
      auto url1 = u1;
      url0.SetPath("/path");
      url1.SetPath("/path");
      EXPECT_EQ(url0.GetAbsoluteUrl(), "https://www.microsoft.com//path"); // <-- fixed in this PR
      EXPECT_EQ(url1.GetAbsoluteUrl(), "https://www.microsoft.com//path"); // <-- fixed in this PR
    }
  }

--
UWP CI is currently blocked until microsoft/vcpkg#35279 is fixed / microsoft/vcpkg#35116 is merged (#5181 would unblock)

@antkmsft antkmsft marked this pull request as ready for review November 26, 2023 04:55
@antkmsft antkmsft added the MQ This issue is part of a "milestone of quality" initiative. label Nov 27, 2023
@antkmsft antkmsft self-assigned this Nov 27, 2023
@antkmsft antkmsft merged commit ee4be19 into Azure:main Nov 28, 2023
40 checks passed
@antkmsft antkmsft deleted the url-doubleslash branch November 28, 2023 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core MQ This issue is part of a "milestone of quality" initiative.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URL class should drop one slash in cases when AppedPath() would end up with two slashes
4 participants