Skip to content
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

Use URL instead of base64-encoded payload for deep link #504

Closed
esune opened this issue May 4, 2024 · 5 comments · Fixed by #557
Closed

Use URL instead of base64-encoded payload for deep link #504

esune opened this issue May 4, 2024 · 5 comments · Fixed by #557
Assignees
Labels
enhancement New feature or request

Comments

@esune
Copy link
Member

esune commented May 4, 2024

When proof-requests grow in size, the resulting deep-link can grow to over 2048 characters, causing the payload to be truncated when sent over to BC Wallet (2048 characters is the size limit for URLs) which will then fail to process the request as the decoded output is a malformed JSON.

One way to fix this would be to use a URL/shortened URL instead of the base64 encoded presentation-request, but we are unsure on whether this will work out-of-the-box or not (attempts seem to indicate this will not work as-is).

@jleach @cvarjao does a change need to get into BC Wallet to handle this, or are we maybe using the bcwallet://...?c_i=https://my-url.com (or similar OOB) syntax incorrectly with the url?

@esune
Copy link
Member Author

esune commented May 15, 2024

Possibly superseeded by #513

@cvarjao
Copy link
Member

cvarjao commented May 15, 2024

I think we need to talk more about invitation URLs and how payloads are retrieved. I still don't fully understand why we process HTTP Header (Location) redirect URL as opposed to just get the payload itself in the response body. And it needs to support deep link (custom protocols)

@esune
Copy link
Member Author

esune commented May 15, 2024

I think we need to talk more about invitation URLs and how payloads are retrieved. I still don't fully understand why we process HTTP Header (Location) redirect URL as opposed to just get the payload itself in the response body. And it needs to support deep link (custom protocols)

This issue was logged specifically for the issue relating to deep-link URLs exceeding the 2048 character limit. If we switch our pattern to using short-lived connections we should be able to also resolve this.

We may still want to chat as I am not 100% sure I follow the location redirect URL and custom protocols bit.

@esune esune moved this from Assignment Ready to Assigned in CDT Enterprise Apps Jun 10, 2024
@esune
Copy link
Member Author

esune commented Jun 10, 2024

Thanks to the BC Wallet team for implementing the new deep-link handler - we should update our code to generate the deep link using the following format: didcomm://?_url=<base64 encoded url>.

@loneil loneil moved this from Assigned to In Progress in CDT Enterprise Apps Jun 11, 2024
@loneil loneil moved this from In Progress to In Review in CDT Enterprise Apps Jun 18, 2024
@loneil
Copy link
Contributor

loneil commented Jun 19, 2024

Handling here #557
BC Wallet release with this is slated to be out next Monday (been testing successfully with beta Android), probably still worth having the config toggle in there though IMO for any needs to troubleshoot it back and forth to the older format or anything for a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants