Skip to content

fix(print): encode queue and task identifiers in URL paths - #453

Draft
jplbrun wants to merge 1 commit into
mainfrom
fix/hasi-print-url-injection
Draft

jplbrun wants to merge 1 commit into
mainfrom
fix/hasi-print-url-injection

Conversation

@jplbrun

@jplbrun jplbrun commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Caller-supplied queue names and task IDs were inserted directly into URL paths, allowing reserved characters to change the request URL. Encode qname and item_id in queue creation, profile retrieval and print-task creation with quote(..., safe=""). Reject literal . and .. segments through the existing HttpError/PrintOperationError handling while preserving identifier spelling and request-body references.

Related Issue

No linked GitHub issue.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • Dependency update

How to Test

Run from the repository root after checking out this branch.

  1. Run the Print unit suite:

    uv sync --frozen --all-extras --group dev
    uv run --frozen pytest tests/print/unit -q

    Expected: 46 passed, including 13 regression cases covering all three URL call sites, dot-segment rejection and UUID spelling preservation.

  2. Compare the old URL construction with the updated client. This example prepares requests with Session.send mocked; it sends no network requests:

    uv run --frozen python - <<'PY'
    from unittest.mock import Mock, patch
    from requests import Request, Response
    from sap_cloud_sdk.core.protocol.http import HttpClient
    from sap_cloud_sdk.print import PrintClient, PrintQueue
    
    base_url = "https://print.example.invalid"
    qname = "../other?param=x#frag"
    raw_url = f"{base_url}/qm/api/v1/rest/queues/{qname}"
    print("Before:", Request("PUT", raw_url).prepare().url)
    
    http = HttpClient(base_url)
    client = PrintClient(http, Mock(), lambda: Mock())
    response = Response()
    response.status_code = 204
    try:
        with patch("requests.Session.send", return_value=response) as send:
            client.create_queue(PrintQueue(qname=qname))
            print("After: ", send.call_args.args[0].url)
    finally:
        http.close()
    PY

    Expected:

    Before: https://print.example.invalid/qm/api/v1/rest/other?param=x#frag
    After:  https://print.example.invalid/qm/api/v1/rest/queues/..%2Fother%3Fparam%3Dx%23frag
    

    Before, .. changes the path, param=x becomes a query, and frag becomes a URL fragment. Fragments are not sent in the HTTP request target. After, those characters remain encoded inside the queue identifier.

  3. Run the repository checks:

    uv run --frozen pre-commit run --files src/sap_cloud_sdk/print/client.py tests/print/unit/test_url_identifiers.py

    Expected: formatting, lint, whitespace and type checks pass. Unchanged YAML and lock-file hooks are skipped.

Checklist

  • I have read the Contributing Guidelines
  • I have verified that my changes solve the issue
  • I have added/updated automated tests to cover my changes
  • All tests pass locally
  • I have verified that my code follows the Code Guidelines
  • I have updated documentation (if applicable)
  • I have added type hints for all public APIs
  • My code does not contain sensitive information (credentials, tokens, etc.)
  • I have followed Conventional Commits for commit messages
  • (New modules only) I have attached e2e evidence showing the module working against a real BTP service — N/A, no new module
  • (Security-sensitive changes only) I have run /security-review and addressed all ❌ findings

Breaking Changes

No breaking changes for documented Print Service usage. Public client signatures, request bodies, and URLs for valid queue names and task UUIDs are unchanged. These callers need no migration.

This security fix changes how other identifier inputs behave:

  • URL delimiters such as /, ?, #, and % are encoded as identifier data instead of being interpreted as URL structure.
  • Literal . and .. identifiers now raise the existing PrintOperationError before an HTTP request.
  • Pre-encoded identifiers are treated literally and encoded again. Callers passing pre-encoded values should pass the original raw identifier instead.

These behavior changes affect inputs outside the service's documented identifier constraints; identical behavior for every previously accepted input is not claimed.

Additional Notes

None

Signed-off-by: João Pedro Brun <joao.pedro.brun@sap.com>

This branch has not been deployed

No deployments
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