fluri icon indicating copy to clipboard operation
fluri copied to clipboard

[BUG] appendToPath should add appropriate separators if missing

Open woelmer opened this issue 4 years ago • 3 comments

  • Issue Type: BUG
  • Dart SDK version(s): 2.8.1
  • fluri Version(s): 1.2.8

With the following example where the base path does not have a trailing path separator, I would expect appendToPath to add the missing separator between the base and the path being appended.

Fluri base = new Fluri('https://example.com/base');

Fluri fluri = new Fluri.from(base)
  ..appendToPath('path/to/resource')

This results in a path of 'https://example.com/basepath/to/resource'


FYI: @dustinlessard-wf @evanweible-wf @jayudey-wf @maxwellpeterson-wf @sebastianmalysa-wf @trentgrover-wf

woelmer avatar May 07 '20 16:05 woelmer

I honestly can't remember if it was implemented like this intentionally or not, but at this point I'm a bit nervous to make this change without a major release in case there are existing usages that rely on this behavior. I do think adding the separator is probably the better option, though, so I'll keep this open and maybe we can find a reason to release a major. I've been meaning to take a look at extension methods now that they're available to see if some of this package's functionality could be an extension on Uri directly, so that would be another reason to release a major.

evanweible-wf avatar May 07 '20 22:05 evanweible-wf

This is a pretty critical bug when your base url comes from a server and you have no idea whether there is a path separator on the end or not (obviously one could check but that defeats the purpose of this library entirely).

woelmer avatar May 08 '20 19:05 woelmer

Totally understand that it's not ideal. But this behavior has existed for over 4 years, so changing this behavior in a patch release would risk breaking existing usages, which in my opinion is worse than this not working as expected for new usages.

evanweible-wf avatar May 08 '20 19:05 evanweible-wf