Skip to content

fix: prevent colons in OData datetime values from being parsed as path parameters#7388

Merged
bijin-bruno merged 1 commit intousebruno:mainfrom
abhishek-bruno:fix/url-colon-parsing
Mar 27, 2026
Merged

fix: prevent colons in OData datetime values from being parsed as path parameters#7388
bijin-bruno merged 1 commit intousebruno:mainfrom
abhishek-bruno:fix/url-colon-parsing

Conversation

@abhishek-bruno
Copy link
Copy Markdown
Member

@abhishek-bruno abhishek-bruno commented Mar 6, 2026

Description

  • Fixes colons in OData datetime values (e.g., T00:00:00) being incorrectly treated as path parameter prefixes, which truncated the URL
  • Changed the OData path parameter regex from /:/g to /:/g to require param names to start with a letter or underscore

Closes #7384

JIRA

Bug

For the URL: https://api10.successfactors.com/odata/v2/EmpJob(seqNumber=1L,startDate=datetime'2021-08-29T00:00:00',userId='213668')?$format=json
The regex /:/g matched :00 from the time string T00:00:00 as a path parameter named 00. During interpolation, these were replaced with empty values, corrupting the URL to: datetime'2021-08-29T00

Fix

Path parameter names are identifiers (:userId, :orderId) — they always start with a letter or underscore, never a digit. The updated regex /:/g enforces this, so :00 in datetime strings is ignored while :userId is still correctly detected.

Contribution Checklist:

  • I've used AI significantly to create this pull request
  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.

Publishing to New Package Managers

Please see here for more information.

Summary by CodeRabbit

  • Bug Fixes

    • Improved path parameter validation to correctly identify valid parameter names in URLs, preventing incorrect interpretation of special characters in values like datetime strings.
  • Tests

    • Added test coverage for datetime values in path parameters to ensure proper handling of OData-style URL segments.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 6, 2026

Walkthrough

URL path parameter parsing is refined across four files to treat colons as parameter delimiters only when followed by identifier-compliant names (starting with a letter or underscore). This prevents datetime expressions in OData-style URLs from being incorrectly parsed as path parameters.

Changes

Cohort / File(s) Summary
Core URL Utilities
packages/bruno-app/src/utils/url/index.js, packages/bruno-app/src/utils/url/index.spec.js
Updated path parameter regex from /[:](\w+)/g to /[:]([a-zA-Z_]\w*)/g in parsePathParams and interpolateUrlPathParams. Added two test cases validating that colons within datetime values are not treated as parameter delimiters.
Runtime Interpolation
packages/bruno-cli/src/runner/interpolate-vars.js, packages/bruno-electron/src/ipc/network/interpolate-vars.js
Applied identical regex constraint to path parameter extraction during CLI and Electron runtime variable interpolation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • PR #6251: Modifies interpolateUrlPathParams in the same file, applying regex-based parameter handling changes.
  • PR #6762: Implements related path parameter detection and interpolation utilities (pathParams/getPathParams), directly adjacent to the parameter parsing logic.

Suggested reviewers

  • lohit-bruno
  • naman-bruno
  • helloanoop

Poem

When colons crept in where they shouldn't belong,
In datetime values singing their song,
A letter must lead, an underscore's fine,
OData URLs now parse just right—no more line! 📅✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: preventing colons in OData datetime values from being incorrectly parsed as path parameters.
Linked Issues check ✅ Passed The PR successfully addresses issue #7384 by updating regex patterns to require parameter names start with letters/underscores, preventing numeric segments like :00 from being treated as parameters.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the OData datetime colon-parsing issue across four files with matching regex updates and supporting test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/bruno-cli/src/runner/interpolate-vars.js (1)

130-169: Consider extracting shared OData path interpolation logic.

The OData path parameter parsing block (lines 130-169) is nearly identical to packages/bruno-electron/src/ipc/network/interpolate-vars.js (lines 154-206). If this logic evolves, keeping them in sync could become error-prone. A shared utility in @usebruno/common could reduce duplication.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bruno-cli/src/runner/interpolate-vars.js` around lines 130 - 169,
The OData-style path interpolation logic inside the interpolatedUrlPath mapping
is duplicated elsewhere — extract it into a shared utility (e.g., create a
function interpolateODataPath(pathSegment, pathParams) in `@usebruno/common`) that
encapsulates the regex check (/^[A-Za-z0-9_.-]+\([^)]*\)$/), the paramRegex
handling, and replacement using request.pathParams (or the passed pathParams
array); then replace the in-file block that uses paramRegex, match loop, and
result replacement with a call to that utility from both this module (where
interpolatedUrlPath is built) and the other consumer to avoid duplication and
keep behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/bruno-cli/src/runner/interpolate-vars.js`:
- Around line 130-169: The OData-style path interpolation logic inside the
interpolatedUrlPath mapping is duplicated elsewhere — extract it into a shared
utility (e.g., create a function interpolateODataPath(pathSegment, pathParams)
in `@usebruno/common`) that encapsulates the regex check
(/^[A-Za-z0-9_.-]+\([^)]*\)$/), the paramRegex handling, and replacement using
request.pathParams (or the passed pathParams array); then replace the in-file
block that uses paramRegex, match loop, and result replacement with a call to
that utility from both this module (where interpolatedUrlPath is built) and the
other consumer to avoid duplication and keep behavior identical.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9cf29f34-0c31-469d-bee0-2bda18df0894

📥 Commits

Reviewing files that changed from the base of the PR and between 5151d29 and eeefe4b.

📒 Files selected for processing (4)
  • packages/bruno-app/src/utils/url/index.js
  • packages/bruno-app/src/utils/url/index.spec.js
  • packages/bruno-cli/src/runner/interpolate-vars.js
  • packages/bruno-electron/src/ipc/network/interpolate-vars.js

@bijin-bruno bijin-bruno merged commit f1b84e0 into usebruno:main Mar 27, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Url parsing failing when colon (:) is present

2 participants