Update buildUrl to prepend webUrlPrefix

[Wolframcheg]

Pull Request Template

Description

Update buildUrl to prepend webUrlPrefix

Type of Change

Bug fix (non-breaking change which fixes an issue)

[pchuri]

Thanks for the PR! The intent behind this fix makes sense — attachment download links are missing the /wiki prefix on instances that use it.

However, I have a concern with modifying toAbsoluteUrl() directly. Since it's a general-purpose utility method, adding webUrlPrefix inside it could cause double-prefixing if the input path already contains /wiki, and may introduce unintended side effects for future callers.

The rest of the codebase handles this at the call site (e.g., lines 416, 510, 1356), explicitly passing webUrlPrefix to buildUrl(). I'd suggest keeping toAbsoluteUrl() unchanged and applying the prefix at the two call sites instead:

// normalizeAttachment (line 2063)
downloadLink: this.toAbsoluteUrl(
  raw._links?.download ? `${this.webUrlPrefix}${raw._links.download}` : null
)

// downloadAttachment (line 864)
downloadUrl = this.toAbsoluteUrl(
  attachment._links?.download ? `${this.webUrlPrefix}${attachment._links.download}` : null
);

This keeps the fix scoped to where it's needed and stays consistent with the existing patterns. What do you think?

[Wolframcheg]

I think your option is better

[adamtan945]

Independently hit this same bug and opened #99 before seeing this PR — apologies for the duplication. Closing #99 in favor of this one since it was opened first. Sharing my root cause analysis and tests here so they're useful for the review.

Root cause

The REST API returns each attachment's _links.download as a path relative to the Confluence context root:

/download/attachments/123456789/example.png?version=1&modificationDate=1700000000000&cacheVersion=1&api=v2

On Atlassian Cloud the context root is /wiki, so the correct URL is https://<domain>/wiki/download/.... Without this patch, toAbsoluteUrl hands the raw path to buildUrl, producing https://<domain>/download/... — which returns 404 on every Cloud instance. That's what breaks confluence attachments --download completely on Cloud (issue #98).

The fix here is correct: prepend webUrlPrefix, which is already computed on line 38 (this.apiPath.startsWith('/wiki/') ? '/wiki' : '') and is an empty string on Server/DC, so this change is backward-compatible.

Verified by curl'ing the two URLs directly against a live Cloud instance with the same credentials:

URL	Result
https://<domain>/download/attachments/.../?version=... (before fix)	HTTP 404
https://<domain>/wiki/download/attachments/.../?version=... (with this fix)	HTTP 200

End-to-end verification

Checked this branch out locally, ran npm install && npm link, then against a real Cloud page:

$ confluence attachments <pageId> --download --dest /tmp/out
⬇️  example.png -> /tmp/out/example.png
Downloaded 1 attachment to /tmp/out

The downloaded file is byte-identical to what you get by fetching _links.download directly with curl against the REST API.

Suggested tests (cherry-pick-able)

I wrote three targeted tests while working on #99 that would pin this fix down. Happy to send them as a follow-up PR if you'd prefer, or you can copy them in here — they're small:

test('toAbsoluteUrl prepends /wiki context path on Atlassian Cloud', () => {
  const cloudClient = new ConfluenceClient({
    domain: 'test.atlassian.net',
    token: 'token',
    apiPath: '/wiki/rest/api'
  });
  expect(cloudClient.toAbsoluteUrl('/download/attachments/123/file.png?version=1&modificationDate=1700000000000&cacheVersion=1&api=v2'))
    .toBe('https://test.atlassian.net/wiki/download/attachments/123/file.png?version=1&modificationDate=1700000000000&cacheVersion=1&api=v2');
});

test('toAbsoluteUrl does not double-prepend /wiki when path already starts with it', () => {
  const cloudClient = new ConfluenceClient({
    domain: 'test.atlassian.net',
    token: 'token',
    apiPath: '/wiki/rest/api'
  });
  expect(cloudClient.toAbsoluteUrl('/wiki/download/attachments/123/file.png'))
    .toBe('https://test.atlassian.net/wiki/download/attachments/123/file.png');
});

test('toAbsoluteUrl leaves Server/DC paths untouched (no webUrlPrefix)', () => {
  const serverClient = new ConfluenceClient({
    domain: 'confluence.example.com',
    token: 'token',
    apiPath: '/rest/api'
  });
  expect(serverClient.toAbsoluteUrl('/download/attachments/123/file.png'))
    .toBe('https://confluence.example.com/download/attachments/123/file.png');
});

One optional hardening

The current one-liner has no guard against _links.download ever starting with /wiki already (it doesn't today, but a future API change could cause /wiki/wiki/...). If you want to be defensive:

const prefix = this.webUrlPrefix || '';
const needsPrefix = prefix && !pathOrUrl.startsWith(`${prefix}/`);
return this.buildUrl(needsPrefix ? `${prefix}${pathOrUrl}` : pathOrUrl);

But the existing one-liner is fine for today's API behavior and matches the style of how webUrlPrefix is used elsewhere in the codebase (lines 416, 510, 1356, 1360 all just concatenate without guards). Totally up to the maintainer.