Skip to content

fix(parse): recognize mixed-case resource URL schemes#3474

Open
patrick-andstar wants to merge 1 commit into
volcengine:mainfrom
patrick-andstar:codex/fix-resource-url-scheme-case
Open

fix(parse): recognize mixed-case resource URL schemes#3474
patrick-andstar wants to merge 1 commit into
volcengine:mainfrom
patrick-andstar:codex/fix-resource-url-scheme-case

Conversation

@patrick-andstar

Copy link
Copy Markdown
Contributor

Description

RFC 3986 defines URI schemes as case-insensitive. Mixed-case HTTP(S) resource URLs were treated as local paths or skipped by accessor routing, and a mixed-case Feishu URL could fall through to the generic HTTP accessor. This change normalizes scheme comparisons only, while preserving the original URL for client calls, metadata, and remote-target validation.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

None. This is a discovery-backed regression fix; no matching open issue was found.

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 not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Recognize mixed-case HTTP(S), Git, and SSH URI schemes in resource accessor and code-hosting routing checks.
  • Route mixed-case Git, HTTP, WebFeed, and Feishu URLs to their intended accessors.
  • Keep the original URL flowing into the existing SSRF guard and add regression coverage for routing and validation.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Executed locally:

  • Focused accessor, code-hosting, and Feishu tests: 174 passed, 4 deselected existing RegistryRouting cases that require a local ov.conf.
  • tests/server/test_api_local_input_security.py -k remote_resource_scheme_is_case_insensitive: 1 passed.
  • tests/misc/test_network_guard.py: 66 passed.
  • Ruff lint, Python bytecode compilation, and git diff --check passed for all changed files.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Not applicable.

Additional Notes

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

Blocking: the UnderstandingAPI route still performs case-sensitive HTTP(S) scheme checks.

When parser_api is enabled, UnifiedResourceProcessor recognizes a mixed-case Feishu URL and bypasses FeishuAccessor, but UnderstandingAPI.parse classifies its candidate with a case-sensitive startswith check. UnderstandingAPI.submit_url has the same check. As a result, an input such as HTTPS://example.larkoffice.com/docx/unit-test-token is recognized as Feishu by FeishuAccessor._is_feishu_url, yet parse treats it as an invalid local path and submit_url rejects it before any network call.

Please make the scheme checks in both UnderstandingAPI.parse and UnderstandingAPI.submit_url case-insensitive, while preserving the original URL passed to the provider, and add regression coverage for both entry points. The existing 23 parser-API tests pass, but none covers this mixed-case path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants