Skip to content

Dart_StringToCString should be removed from the Dart API and replaced with safer equivalents #2857

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

Closed
lexprfuncall opened this issue May 2, 2012 · 9 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P3 A lower priority bug or feature request

Comments

@lexprfuncall
Copy link

As Dart strings are permitted to contain an arbitrary number of U+0000 code points, it is not possible to safely and securely marshal Dart strings into C strings.

I am in the process of adding Dart_StringToBytes which takes a Dart string and returns an array of UTF-8 code units and a length. I believe this function, along with a function to compare a Dart string with an array of UTF-8 code units, should be sufficient to replace the uses of Dart_StringToCString.

@dgrove
Copy link
Contributor

dgrove commented May 2, 2012

Added Area-VM, Triaged labels.

@whesse
Copy link
Contributor

whesse commented May 2, 2012

Dart_StringToCString is used very often in runtime/bin, directly and through DartUtils::GetStringValue. If it is replaced with a safer variant, which I agree is probably necessary, then a common use case will be:

Turn a Dart String into a UTF8 C null-terminated string, throwing an error if there are null characters in the Dart String.

Can we support that efficiently? Could we keep a StringToCString that returns an error if there is a null in the string?

We really need something that outputs a null-terminated string. Could StringToBytes add a null character at the end?

@iposva-google
Copy link
Contributor

Why can't Dart_StringToCString just return the UTF-8 encoded string value? I understand that you could still have a NUL character in your Dart string and thus you would get a short string from the C perspective, but at least you get something that is very, very convenient from the usage perspective.

@iposva-google
Copy link
Contributor

Set owner to @lexprfuncall.

@iposva-google
Copy link
Contributor

Removed the owner.

@iposva-google
Copy link
Contributor

Added this to the Later milestone.
Removed Priority-Medium label.
Added Priority-Low label.

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@lexprfuncall lexprfuncall added Type-Defect P3 A lower priority bug or feature request area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. labels Aug 4, 2014
@iposva-google
Copy link
Contributor

We decided that the convenience trumps the concern of short C strings. Keeping this as is, returning a UTF-8 string.

@kevmoo kevmoo removed the triaged label Mar 1, 2016
dart-bot pushed a commit that referenced this issue Sep 14, 2021
New commits in this revision:

git log --format=%C(auto) %h %s cafadd17ba285dad9ebe6d34c617bc0d70d096b3..98a01e1cdbf441f030f1be76417860b6b372663d ✓
 cafadd17 Don't use null safety (yet) (#3112)
 98a01e1c Store credentials in a config directory (#3092)
 4ee280b7 Fixed messages (#3110)
 bbdac802 Third party hosted pub registry authentication (#3007)
 b1bedc53 `--examples` for get/upgrade/downgrade/add/remove (#2857)

Change-Id: Ibf6ffe33820c97114958c1564a7c22a46ff73d9c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/213348
Commit-Queue: Sigurd Meldgaard <[email protected]>
Reviewed-by: Jonas Jensen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

6 participants