Skip to content

fix: joinUrl no longer collapses the URL scheme#2915

Merged
vmaerten merged 2 commits into
go-task:mainfrom
vsaraikin:fix/joinurl-scheme
Jul 14, 2026
Merged

fix: joinUrl no longer collapses the URL scheme#2915
vmaerten merged 2 commits into
go-task:mainfrom
vsaraikin:fix/joinurl-scheme

Conversation

@vsaraikin

Copy link
Copy Markdown
Contributor

What

joinUrl collapses the // in a URL scheme, so it produces an invalid URL:

vars:
  SERVER: "http://localhost"
tasks:
  default:
    cmds:
      - echo "{{joinUrl .SERVER "path1" "path2"}}"
      # got:  http:/localhost/path1/path2
      # want: http://localhost/path1/path2

The documented behavior (docs/reference/templating.md, "URLs" section) shows http://localhost/path1/path2, so this is a clear bug — and it's the whole reason joinUrl exists separately from joinPath.

Why

joinUrl was implemented as path.Join(elem...). path.Join runs path.Clean, which collapses consecutive slashes and destroys the // after the scheme.

Fix

Use net/url.JoinPath, which is purpose-built for joining URL path segments and preserves the scheme. The signature becomes (string, error), consistent with the sibling relPath/absPath funcs (filepath.Rel/filepath.Abs), and the template engine already supports funcs that return an error.

Tests

Added TestJoinUrl in internal/templater/funcs_test.go covering scheme preservation, a base with an existing path, and trailing-slash handling. joinUrl shipped without a test in #2408, which is how this slipped through.

joinUrl used path.Join, whose path.Clean collapses the // in a URL scheme,
so {{joinUrl "http://localhost" "p1" "p2"}} produced http:/localhost/p1/p2
instead of the documented http://localhost/p1/p2. Use net/url.JoinPath, which
preserves the scheme.

@trulede trulede 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.

Seems OK.

@vmaerten

Copy link
Copy Markdown
Member

@vmaerten vmaerten self-requested a review July 14, 2026 20:02
@vsaraikin

Copy link
Copy Markdown
Contributor Author

Done — added t.Parallel() to TestJoinUrl and its subtests to satisfy the paralleltest linter. Thanks!

@vmaerten vmaerten enabled auto-merge (squash) July 14, 2026 20:25
@vmaerten vmaerten merged commit f6f458f into go-task:main Jul 14, 2026
14 checks passed
vmaerten added a commit that referenced this pull request Jul 14, 2026
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.

3 participants