Skip to content

chore: Preview for removing encoding from Phone NUmber format #871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

manisha1997
Copy link
Contributor

@manisha1997 manisha1997 commented Jul 1, 2025

Fixes #793

Preview for twilio/twilio-oai-generator#657

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

Copy link

sonarqubecloud bot commented Jul 1, 2025

Copy link
Contributor

@tiwarishubham635 tiwarishubham635 left a comment

Choose a reason for hiding this comment

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

LGTM! Verified affected APIs by making calls

manisha1997 added a commit to twilio/twilio-oai-generator that referenced this pull request Jul 2, 2025
…mat (#657)

<!--
We appreciate the effort for this pull request but before that please
make sure you read the contribution guidelines, then fill out the blanks
below.

Please format the PR title appropriately based on the type of change:
  <type>[!]: <description>
Where <type> is one of: docs, chore, feat, fix, test, misc.
Add a '!' after the type for breaking changes (e.g. feat!: new breaking
feature).

**All third-party contributors acknowledge that any contributions they
provide will be made under the same open-source license that the
open-source project is provided under.**

Please enter each Issue number you are resolving in your PR after one of
the following words [Fixes, Closes, Resolves]. This will auto-link these
issues and close them when this PR is merged!
e.g.
Fixes #1
Closes #2
-->

# Fixes #

This PR removes encoding in case of phone number format because all path
params are anyway encoded
https://github.com/twilio/twilio-java/blob/4f46e707d6931c50888c26b2dbd3c4452ea076e1/src/main/java/com/twilio/http/Request.java#L213

Preview PR: twilio/twilio-java#871


### Checklist
- [x] I acknowledge that all my contributions will be made under the
project's license
- [ ] Run `make test-docker`
- [ ] Verify affected language:
- [ ] Generate [twilio-go](https://github.com/twilio/twilio-go) from our
[OpenAPI specification](https://github.com/twilio/twilio-oai) using the
[build_twilio_go.py](./examples/build_twilio_go.py) using `python
examples/build_twilio_go.py path/to/twilio-oai/spec/yaml
path/to/twilio-go` and inspect the diff
    - [ ] Run `make test` in `twilio-go`
    - [ ] Create a pull request in `twilio-go`
    - [ ] Provide a link below to the pull request
- [ ] I have made a material change to the repo (functionality, testing,
spelling, grammar)
- [ ] I have read the [Contribution
Guidelines](https://github.com/twilio/twilio-oai-generator/blob/main/CONTRIBUTING.md)
and my PR follows them
- [ ] I have titled the PR appropriately
- [ ] I have updated my branch with the main branch
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added the necessary documentation about the functionality
in the appropriate .md file
- [ ] I have added inline documentation to the code I modified

If you have questions, please create a GitHub Issue in this repository.

Co-authored-by: twilio-dx <[email protected]>
@manisha1997 manisha1997 requested a review from Copilot July 22, 2025 17:04
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes UTF-8 encoding from phone number formatting across multiple Twilio SDK files, replacing .encode("utf-8") calls with .toString() calls on phone number objects used in URL path construction.

  • Replaces phone number encoding method calls with toString() calls
  • Affects multiple API endpoints across trusthub, studio, pricing, numbers, and core API modules
  • Standardizes phone number formatting approach across the SDK

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ComplianceTollfreeInquiriesCreator.java Updates toll-free phone number path parameter formatting
ExecutionCreator.java (v1/v2) Updates To/From phone number path parameters in Studio flow executions
EngagementCreator.java Updates To/From phone number path parameters in Studio engagements
NumberFetcher.java (pricing v1/v2) Updates destination/phone number path parameters in pricing APIs
HostedNumberOrderCreator.java (numbers/preview) Updates phone number and contact phone number path parameters
AuthorizationDocumentCreator.java Updates contact phone number path parameter
PortingPortabilityFetcher.java Updates phone number path parameter
TollFreeCreator.java/MobileCreator.java/LocalCreator.java Updates phone number path parameters for incoming phone number creation
ValidationRequestCreator.java Updates phone number path parameter for validation requests
MessageCreator.java Updates To phone number path parameter for message creation

@@ -259,7 +259,7 @@ public ComplianceTollfreeInquiries create(final TwilioRestClient client) {
path =
path.replace(
"{" + "TollfreePhoneNumber" + "}",
this.tollfreePhoneNumber.encode("utf-8")
this.tollfreePhoneNumber.toString()
Copy link
Preview

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

Removing UTF-8 encoding from phone numbers in URL paths may cause issues with special characters or international phone numbers. Phone numbers containing special characters like '+' or spaces should be properly URL-encoded when used in path parameters to prevent URL parsing errors.

Copilot uses AI. Check for mistakes.

Comment on lines +80 to +81
path = path.replace("{" + "To" + "}", this.to.toString());
path = path.replace("{" + "From" + "}", this.from.toString());
Copy link
Preview

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

Phone numbers in URL paths should be properly encoded to handle special characters like '+' and spaces. Using toString() instead of encode("utf-8") may cause URL parsing errors for international phone numbers.

Suggested change
path = path.replace("{" + "To" + "}", this.to.toString());
path = path.replace("{" + "From" + "}", this.from.toString());
path = path.replace("{" + "To" + "}", URLEncoder.encode(this.to.toString(), "UTF-8"));
path = path.replace("{" + "From" + "}", URLEncoder.encode(this.from.toString(), "UTF-8"));

Copilot uses AI. Check for mistakes.

path = path.replace("{" + "To" + "}", this.to.encode("utf-8"));
path = path.replace("{" + "From" + "}", this.from.encode("utf-8"));
path = path.replace("{" + "To" + "}", this.to.toString());
path = path.replace("{" + "From" + "}", this.from.toString());
Copy link
Preview

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

Phone numbers in URL paths should be properly encoded to handle special characters like '+' and spaces. Using toString() instead of encode("utf-8") may cause URL parsing errors for international phone numbers.

Copilot uses AI. Check for mistakes.

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.

NumberFetcher is double encoding the number
3 participants