Bug #2015
closed
name.toUri() does not return a valid URI: missing scheme
Added by Felix Rabe over 9 years ago. Updated about 9 years ago.
0%
Description
I just noticed that Name.toUri() returns "/some/path" instead of "ndn:/some/path".
The scheme part is not optional: https://tools.ietf.org/html/rfc3986#section-3
Updated by Anonymous over 9 years ago
- Assignee set to Anonymous
You're right that a URI should strictly have ndn: in front. We really use toUri() to "pretty print" a name and usually don't want to see the "ndn:". There is a lot of code out there so I think we need to keep the same behavior for backwards compatibility.
What do you think of adding a second boolean argument includeScheme (default false for backwards compatibility), and we clearly document that the default is strictly not a full URI?
Luckily, the library does allow "ndn:" when creating a name from a URI, so you can do new Name("ndn:/some/path"). So it would have symmetric support for to/from URI.
Updated by Felix Rabe over 9 years ago
Semver: "Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable."
As this has not (officially) been pushed to NPM yet, I assume most code is under control by the NDN project somehow? So a half-manual search-and-replace (e.g. renaming toUri to toPath) would be feasible and not too much work.
Boolean arguments for functions are considered harmful (yes, in theory allowed, but in practice please tell me what name.toUri() does differently from name.toUri(true) without looking at either its source code nor its documentation). There is a famous (and I think humorous) article about this topic, but I can't think of a term to throw into Google right now to look it up.
To summarize in maybe more coherent terms, this is my proposal:
Rename
toUritotoPathand all references to it while backwards-compatibility is not yet a (bigger) issue.Change
toUrito return a valid URI.
Updated by Anonymous over 9 years ago
The toUri method is used not just be the JavaScript library, but the C++, Python and Java libraries. And toUri is also used in ndn-cxx so that NFD can print routing prefixes, etc. (where you don't want to see "ndn:").
This would be a big across-the-board change which affects the whole community and a lot of existing code in many languages. If you feel strongly, this is large enough that I would suggest you send an email to ndn-lib to bring the issue to the community.
Updated by Anonymous over 9 years ago
... or we can add an extra default parameter to toUri.
Updated by Anonymous over 9 years ago
I would be favorable to the idea that we should introduce toPath and deprecate toUri. (We can also add something like toNdnUri which includes the "ndn:".) It is still a big enough change that I think we should discuss with the community on ndn-lib. I say this because a major design goal of the common client libraries is stability of the API, even if we are in alpha mode.
Updated by Anonymous about 9 years ago
- Status changed from In Progress to Closed
Following note 1, added an optional param includeScheme.