search

NET-A-PORTER / scala-uri

Simple scala library for building and parsing URIs
Other
261 stars 33 forks source link

Added a filter to filter out empty path parts. #37

Closed Falmarri closed 10 years ago

Falmarri commented 10 years ago

This fixes an error where a string parsed to a URI without a path part got an empty pathpart. This resulted in an extra "/" being put at the begging of the query. That is:

scala> ("http://test.com:8080" / "something").toString res10: String = http://test.com:8080//something

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 89d1f39de2bc13b2245e6f24840e2a094a1c8d3d on Falmarri:0.4.x into 3009337643e53034e27f4956a7cbba63ddc7d3c3 on NET-A-PORTER:0.4.x.

theon commented 10 years ago

Thanks!

Your example definitely needs to be fixed, but I'm not sure we should do it by filtering out all empty path parts. If someone parses a URL "http://www.example.com/hi//bye", I don't think we should flatten it to "http://www.example.com/hi/bye"

WDYT?

Falmarri commented 10 years ago

That's a good question. I think in general, normalizing the path would be OK. I think it would be a pretty esoteric use case to want "//" in a url, I can't think of a time I've ever seen that in the wild.

Maybe instead of filter use dropWhile?

theon commented 10 years ago

I agree it is esoteric, but since it is a valid URL against the spec I would like to support it.

I have pushed a fix and some unit tests in 06ee51f and published a new 0.0.4-SNAPSHOT. Could you give this a test?

Falmarri commented 10 years ago

Yeah that fixed my specific issue.