Skip to content

Comments

Replace urljoin with string concatenation for base URL construction#2867

Merged
adamtheturtle merged 8 commits intomainfrom
adamtheturtle/fix-duplicate-fixture
Feb 22, 2026
Merged

Replace urljoin with string concatenation for base URL construction#2867
adamtheturtle merged 8 commits intomainfrom
adamtheturtle/fix-duplicate-fixture

Conversation

@adamtheturtle
Copy link
Member

@adamtheturtle adamtheturtle commented Feb 22, 2026

Summary

  • urljoin(base, path) silently drops any path prefix from base_vws_url when path starts with / — so base_vws_url="http://localhost/mock" would have /mock silently ignored
  • String concatenation (base_vws_url.rstrip("/") + request_path) preserves the full base URL path as intended

Test plan

  • Existing tests continue to pass (no functional change for the default https://vws.vuforia.com base URL or path-less mock URLs)

🤖 Generated with Claude Code


Note

Low Risk
Small change to URL assembly plus targeted tests; behavior should only differ for callers using custom base URLs with path prefixes.

Overview
Fixes URL construction for both VWS and VWQ requests by replacing urljoin with base_url.rstrip("/") + request_path, preventing a custom base URL’s path prefix from being silently dropped.

Adds regression tests covering custom base URLs with path prefixes for VWS (list_targets) and CloudRecoService (query), and bumps the dev dependency vws-python-mock to 2026.2.22.2 to support these cases.

Written by Cursor Bugbot for commit 13a0047. This will update automatically on new commits. Configure here.

urljoin(base, path) silently drops any path prefix from the base URL
when path starts with '/', so base_vws_url values like
'http://localhost/mock' would have '/mock' ignored. String concatenation
preserves the full base URL path as intended.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Apply the same urljoin → string concatenation fix to query.py
  (CloudRecoService had the identical bug)
- Add TestCustomBaseVWSURL::test_custom_base_url_with_path_prefix
  and TestCustomBaseVWQURL::test_custom_base_url_with_path_prefix
  to assert that a base URL containing a path prefix is preserved
  in the outgoing request URL; these tests would fail with the old
  urljoin approach

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
adamtheturtle and others added 2 commits February 22, 2026 09:50
Instead of using responses directly, set up MockVWS at the bare URL
and assert ConnectionError when the client uses a path-prefix URL.
With the old urljoin code the prefix would be silently dropped so
the request would succeed; with string concatenation the prefix is
preserved, the mock has no handler for it, and ConnectionError is
raised as expected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The new release fixes the urljoin bug in MockVWS so it correctly
registers handlers at path-prefixed base URLs. The path-prefix tests
stay as ConnectionError tests: MockVWS listens at the bare URL while
the client uses the prefixed URL, so the prefix being preserved
causes a ConnectionError -- the inverse of the urljoin behaviour where
the prefix would be silently dropped and the mock would respond.

(A fully positive round-trip test is not yet possible because the HMAC
signing also needs to incorporate the base URL path prefix, which is a
separate issue.)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… URLs

- Replace urljoin with string concatenation in both _vws_request.py and
  query.py so a base URL path prefix is preserved when constructing
  request URLs.
- Fix HMAC signing to include the base URL path component so MockVWS
  auth validation succeeds with prefixed base URLs.
- Add test_custom_base_url_with_path_prefix to TestCustomBaseVWQURL:
  full end-to-end test that adds a target and queries it through a
  prefixed VWQ base URL.
- Add test_custom_base_url_with_path_prefix to TestCustomBaseVWSURL:
  verifies the prefixed URL reaches MockVWS; suppresses UnknownTargetError
  pending VWS-Python/vws-python-mock#2995.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@adamtheturtle adamtheturtle merged commit 545e91d into main Feb 22, 2026
16 checks passed
@adamtheturtle adamtheturtle deleted the adamtheturtle/fix-duplicate-fixture branch February 22, 2026 20:40
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.

1 participant