lemonlabsuk / scala-uri

Simple scala library for building and parsing URIs
Other
305 stars 34 forks source link

Url parsing regression. #21

Closed garriguv closed 6 years ago

garriguv commented 6 years ago

Hi,

We're trying to update scala-uri to the latest version (from 0.4.x), and we're running into a regression for some URLs (admittedly, pretty strange ones 😄):

soundcloud://sounds:78237871?target_url=https%3A%2F%2Fsoundcloud.com%2Fosfdjskj-dkfsdfa%2Fofsfhsdlfsf

Interestingly, this works in the scala js example website: https://lemonlabs.io/scala-uri-scalajs-example/index.html

This likely got introduce between 1.0.0-rc1 and 1.1.4.

Code Example:

val url = Url.parse("soundcloud://sounds:476949372?target_url=https%3A%2F%2Fsoundcloud.com%2Fosvaldo-j-figueroa%2Fosvval-tat-freestyle")

Results in the following:

[info] - should result in a valid Uri object *** FAILED ***
[info]   io.lemonlabs.uri.parsing.UriParsingException: Invalid URL could not be parsed. Unexpected end of input, expected [^@] or '@' (line 1, column 102):
[info] soundcloud://sounds:78237871?target_url=https%3A%2F%2Fsoundcloud.com%2Fosfdjskj-dkfsdfa%2Fofsfhsdlfsf
[info]                                                                                                      ^
[info]   at io.lemonlabs.uri.parsing.UrlParser.getOrThrow(UrlParser.scala:230)
[info]   at io.lemonlabs.uri.parsing.UrlParser.parseUrl(UrlParser.scala:274)
[info]   at io.lemonlabs.uri.parsing.UrlParser$.parseUrl(UrlParser.scala:325)
[info]   at io.lemonlabs.uri.Url$.parse(Uri.scala:545)
[info]   at io.lemonlabs.uri.ParsingTests.$anonfun$new$7(ParsingTests.scala:61)
[info]   at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info]   at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info]   at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info]   at org.scalatest.Transformer.apply(Transformer.scala:22)
[info]   at org.scalatest.Transformer.apply(Transformer.scala:20)
[info]   ...

I'm having a look now, but maybe you know exactly where to look 😃

theon commented 6 years ago

Thanks for reporting this! Sorry for the delay replying; I'll take a look tonight.

theon commented 6 years ago

The error message isn't very good here, but the issue is the port number. In 2dc30fbea4076e50f3d4129f062f03e81ebf6754 the port number was limited to being a maximum five characters (since TCP only supports port numbers up to to 65535).

I am happy to remove this restriction, since there is little benefit. Note however that port number will still be limited to Int.MaxValue

theon commented 6 years ago

Thanks again for reporting this. I have released version 1.1.5 with the original behaviour where port numbers up to Int.MaxValue are supported.

garriguv commented 6 years ago

That was super fast. Thank you :+1: