Skip to content

Conversation

grdsdev
Copy link
Contributor

@grdsdev grdsdev commented Oct 13, 2025

🔍 Description

Change httpEndpointURL to manipulate url using URL JavaScript type.

What changed?

No behavior change, just use correct types for url manipulation instead of string-based manipulation.

Why was this change needed?

To avoid possible security issues with url manipulation.

🔄 Breaking changes

  • This PR contains no breaking changes

📋 Checklist

  • I have read the Contributing Guidelines
  • My PR title follows the conventional commit format: <type>(<scope>): <description>
  • I have run npx nx format to ensure consistent code formatting
  • I have added tests for new functionality (if applicable)
  • I have updated documentation (if applicable)

📝 Additional notes

Remove string-based URL manipulation in favor of URL object
@grdsdev grdsdev requested review from a team as code owners October 13, 2025 13:13
@coveralls
Copy link

coveralls commented Oct 13, 2025

Coverage Status

coverage: 95.455% (+12.8%) from 82.621%
when pulling 2d8f0b5 on guilherme/realtime-url-manipulation
into 30d8caa on master.

Copy link
Contributor

@mandarini mandarini left a comment

Choose a reason for hiding this comment

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

Nit: I think that wsUrl.pathname === '' pathname here will never be an empty string, but no harm done in leaving this check there!

@grdsdev
Copy link
Contributor Author

grdsdev commented Oct 13, 2025

Nit: I think that wsUrl.pathname === '' pathname here will never be an empty string, but no hard done in leaving this check there!

Why it can't be empty @mandarini ?

@mandarini
Copy link
Contributor

Why it can't be empty @mandarini ?

As mentioned in the pathname spec, URLs with hierarchical schemes (ws URLs follow a hierarchical scheme) "always have at least one (invisible) path segment: the empty string. The pathname value for such URLs will therefore always have at least one / character.".

We can leave the empty check though in any case!

@grdsdev grdsdev merged commit bced454 into master Oct 13, 2025
22 checks passed
@grdsdev grdsdev deleted the guilherme/realtime-url-manipulation branch October 13, 2025 19:53
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.

4 participants