launchUrl - Invalid argument: url not http(s) crash

[aaravgarg]

Summary

• Validate URL scheme (http/https only) before calling launchUrl in markdown renderers
• Prevents crash when user-provided markdown content contains non-HTTP URLs (e.g. javascript:, data:, tel:, or malformed URLs)
• Fixed in both chat markdown widget and app markdown viewer

Crash Stats

• Events: 131 (iOS) + 75 (Android)
• Users affected: 24 (iOS) + 19 (Android)
• Crashlytics (iOS): View in Console
• Crashlytics (Android): View in Console

Test plan

• Verify markdown links with http/https still work
• Test markdown with non-standard URL schemes (should be ignored, not crash)

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request effectively addresses the crash caused by launchUrl with non-HTTP(S) URLs by adding validation for the URL scheme in both markdown_viewer.dart and markdown_message_widget.dart. The changes are correct and prevent the reported crash.

I've added one comment in markdown_viewer.dart pointing out a latent bug in the existing logic for appending a uid to URLs. While your change fixes the crash, the URL modification logic itself is fragile and can break URLs containing fragments. I've suggested a more robust implementation using Dart's Uri class to handle URL manipulations safely. The change in markdown_message_widget.dart looks good.

[gemini-code-assist]

While this change correctly prevents the crash by validating the URL scheme, the way the uid query parameter is added on the preceding lines is fragile and can lead to broken URLs. Modifying the URL via string concatenation doesn't account for URL fragments (#).

For example, a URL like http://example.com/page#section would be incorrectly transformed into http://example.com/page#section?uid=..., which is an invalid URI structure.

A more robust approach is to parse the URL into a Uri object first, and then use its methods to add query parameters. This ensures that all parts of the URL (including fragments) are handled correctly.

Consider refactoring the onTapLink handler like this:

onTapLink: (text, href, title) {
  if (href == null) return;

  var uri = Uri.tryParse(href);
  if (uri == null) return;

  // Only handle http and https schemes.
  if (uri.scheme == 'http' || uri.scheme == 'https') {
    // Add uid query parameter robustly.
    final newQueryParameters = Map<String, dynamic>.from(uri.queryParameters);
    newQueryParameters['uid'] = SharedPreferencesUtil().uid;
    final newUri = uri.replace(queryParameters: newQueryParameters);
    launchUrl(newUri);
  }
}

This would make the link handling logic safer and prevent future issues.

[mdmohsin7]

closed in #4628

[github-actions]

Hey @aaravgarg 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

• Project standards — Ensuring consistency across the codebase
• User needs — Making sure changes align with what our users need
• Code best practices — Maintaining code quality and maintainability
• Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜