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

Strange behaviour of Uri.dataFromBytes() constructor for different MIME types #28592

Closed
sgrekhov opened this issue Feb 1, 2017 · 2 comments
Closed
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-a library-core type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@sgrekhov
Copy link
Contributor

sgrekhov commented Feb 1, 2017

main() {
  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "").path); // ;base64,AAEC/w==
  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "").data.mimeType); // text/plain

  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: null).path); // ;base64,AAEC/w==
  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: null).data.mimeType); // text/plain

  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "text/plain").path); // ;base64,AAEC/w==
  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "text/plain").data.mimeType); // text/plain

  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "Text/Plain").path); // Text/Plain;base64,AAEC/w==
  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "Text/Plain").data.mimeType); // Text/Plain

  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "Avada / Kedavra").path); // Avada%20/%20Kedavra;base64,AAEC/w==
  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "Avada / Kedavra").data.mimeType); // Avada / Kedavra

  print(new Uri.dataFromBytes([0, 1, 2, 255], mimeType: "Text").path); // Unhandled exception:Invalid argument (mimeType): Invalid MIME type: "Text"
}

Why empty, null and "text/plain" MIME types are treated as "text/plain" but not added to the path component of Uri? Why empty and null mimeType are not treated as invalid ones?

Please also add an expected behaviour for different MIME types to the Uri.dataFromBytes() documentation at https://api.dartlang.org/stable/1.21.1/dart-core/Uri/Uri.dataFromBytes.html

@lrhn
Copy link
Member

lrhn commented Feb 1, 2017

Arguably, passing null as mime-type should act like not passing anything, but Uri.dataFromBytes has a default value which gets overridden by the null, and then it falls back to the default for data: URIs, which is text/plain. That should be fixed.

Another problem is that Uri is really built for hierarchical URIs, and data: URIs are not hierarchical. That means that most getters on Uri will return meaningless values because it treats the data string as a path (and possibly query and fragment). We can embed a data URI in a Uri object, but only really as a way to get it from A to B. To use it properly, you should use the data getter.

The meaning of "" and "text/plain" as a MIME type is the same thing. The default MIME-type for data URIs is text/plain, so that is what an omitted (empty) mime type means. We don't include the default mime type in the resulting URI text since it's implicit anyway.

Mime types and property names are not case normalized. We could probably do that - but tradition actually seems to be to keep some subtypes capitalized. At least we should recognize Text/Plain as equivalent to text/plain and omit both.
We don't try to recognize the registered names, so anything with a single embedded / is allowed (which is why the last line throws). Any invalid characters are escaped, just to be permissive.

So there are two real issues here:

  • null should mean the same as the default value (we should just not use a default value, and instead do a mimeType ??= "application/octet-stream"; inside the body).
  • Other capitalizations of the default mime-type should also be recognized.

Documentation should be improved for the remaining cases.

@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Feb 1, 2017
@lrhn
Copy link
Member

lrhn commented Oct 20, 2021

You can no longer pass null as argument to mimeType, it's a non-nullable optional parameter now.

Another issue here is that we only recognize text/plain when using the mimeType argument, not in UriData._parse.
That means Uri.parse("data:text/plain,") will not give the same result as UriData.fromString("", mimeType: "text/plain"). The latter will convert text/plain to an empty string (because it's the default), but the former won't change the original string at all.
That's inconsistent (but cheap, changing the string costs extra).

So, we can change the UriData.fromString mimeType parameter documentation and behavior, but we probably don't want to fiddle with the parse function (we can, we just don't want to spend time changing the string that was passed in unless we really, really need to).

A third issue is that we don't even recognize TEXT/PLAIN if it's not lower-case and passed to fromString. That we can fix.
The rest should probably just be documented.

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. core-a library-core type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants