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

Uri.origin should never throw #55997

Open
giregk opened this issue Jun 13, 2024 · 2 comments
Open

Uri.origin should never throw #55997

giregk opened this issue Jun 13, 2024 · 2 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug

Comments

@giregk
Copy link

giregk commented Jun 13, 2024

I believe Uri.origin should never throw.

  • When host is empty or null, it should return ://
  • When scheme is empty or null, it should return
  • When both host and scheme are empty or null, it should return an empty string or null

If you disagree with me on these values, then at least Uri.origin should always return null instead of throwing.

Indeed, it is not clear when we use .origin prop on a URI that it can throw which leads to bugs.

My use case : I am writing a password manager, so I have to deal with unknown URIs (such as URIs imported from other tools).
I consider that anything that Uri.tryParse can parse is a valid URI (which seems fair right ?)
I discovered that .origin could throw, which I was of course able to fix by testing properties. But in the future, it is very probable I will forget about this peculiarity and create another bug using .origin an a URI without testing scheme and host first, which is why I filed this issue.

This would be a breaking change. I'm not sure about its impact, but I would say it's easily manageable.

dart version : 3.4.3

Copy link

area-core-library, type-enhancement

The Uri.origin method throws an exception when the scheme or host properties are empty or null. This behavior is unexpected and can lead to bugs, especially when dealing with unknown URIs. The user proposes that Uri.origin should always return a value, even if the scheme or host is missing, to avoid unexpected exceptions.

@github-actions github-actions bot added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug labels Jun 13, 2024
@lrhn
Copy link
Member

lrhn commented Jun 14, 2024

It's not technically a breaking change to make something not throw an error, but if the current origin behavior is being used as a way to check if two different URIs have the same origin, then returning an empty string instead of an error may cause false positives.

The Dart Uri.origin matches neither the HTML behavior nor the WhatWG URL behavior, so there is no real specification that it's failing to follow. It's not even trying.

I think returning the string "null" would be a reasonable (and semi-compliant) behavior. Maybe we should do that.

@lrhn lrhn added library-core and removed triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

2 participants