BROOKLYN-421: Adds DSL for $brooklyn:urlEncode(...)

[aledsage]

As stated in the comments within the code, the url encoding is based on "x-www-form-urlencoded". Therefore care must be taken if encoding username or password (e.g. in http://myuser:mypass@myhost"). It will not encode space correctly, and will not escape "*". The latter we can probably live with, but the former will be wrong.

[Graeme-Miller]

lgtm

[drigodwin]

Looks fine to me as well, it would be good to have brooklyn-docs PR for this too. Thanks @aledsage!

[neykov]

URLEncoder.encode is used for encoding form data for submission in the request body with a x-www-form-urlencoded Content-Type. That's different from URL encoding.
Here's a relevant SO link. See also the last downvoted answer.
We should deprecate Urls.encode and use a proper URL builder instead. This one I like for being small with no dependencies.

[aledsage]

@neykov agreed - I stayed well clear of URLEncoder originally for that reason, and tried to use guava's https://google.github.io/guava/releases/18.0/api/docs/com/google/common/net/UrlEscapers.html. However, it turns out that fragment and path escaping weren't good enough for the use-case in BROOKLYN-421 (e.g. encoding username:password within a url for basic auth). I eventually gave in when I saw that we already called URLEncoder in our Urls utility class.

The https://github.com/mikaelhg/urlbuilder library looks interesting and promising. Not sure how well it would handle the username:password use-case though, particularly given these bits are being passed in the DSL and constructed into a string using $brooklyn:formatString. Also, even if we did know which part was which, how does https://github.com/mikaelhg/urlbuilder/blob/master/src/main/java/io/mikael/urlbuilder/UrlBuilder.java#L314-L319 know if it's got a username with a colon in or a username:password, given it takes a single string?! I guess it just assumes that colon doesn't need to be escaped (see https://github.com/mikaelhg/urlbuilder/blob/master/src/main/java/io/mikael/urlbuilder/util/Encoder.java#L111).

[neykov]

I guess yes - the colon is not escaped @aledsage.

I'm thinking we would expose the builder in the DSL, not use it piecemeal to stitch together a URL in formatString. So it would look something like $brooklyn:urlBuilder("http://host/path.to.jar").userInfo(external("artifactory.credentials")).build(). This is now possible to code with the deferred execution DSL. Not sure yet how templating would work with that - needs some more thought.

[geomacy]

+1 to the idea of exposing the builder, which makes it unambiguous as to how to interpret each bit. I think if you do that then you can get away without the external library and just use java.net.URI,

The multi-argument constructors quote illegal characters as required by the components in which they appear. The percent character ('%') is always quoted by these constructors. Any other characters are preserved.

I know I used it before to do character escaping, but don't remember the details, perhaps my use case was simpler.

[neykov]

@geomacy It's not completely clear to me. If one of the query values contains a & how does the escaping work with the URI constructor? For example q1=my&value&q2=my-value.

[geomacy]

I think it only escapes illegal characters, not reserved ones, so q1=my&value&q2=my-value won't be changed - it assumes you know what you are doing. Let's see...

This passes:

    @Test
    public void testQuery() throws Exception {
        String tester = "q1=my&value&q2=my-value";
        final URI uri = new URI(null, null, null, tester, null);
        final String val = uri.getQuery();
        assertEquals(val, tester);
    }

[geomacy]

I hesitate to suggest this but one "creative" use of URI could be to do the encoding along the lines of the above, e.g.

    @Test
    public void testURLEncode() throws Exception {
        final String str = "http://myusername@super secret:myhost";
        final URI u3 = new URI(null, str, null);
        final String asciiString = u3.toASCIIString();
        assertEquals(asciiString, "http://myusername@super%20secret:myhost");
    }

[geomacy]

I'm not really suggesting we do this by the way, just musing; it's too creative by half for me! I still stick with my point above agreeing that it's best to expose a URL builder DSL - by being explicit about each part it means you can interpret each value you are given very precisely.

[geomacy]

Actually I take it back - having looked more carefully at the spec for that j.n.URI constructor I think the above is a perfectly valid use.

[aledsage]

@geomacy minor correction - separator between username and password is :, and then @ before the hostname.

Unfortunately the URI encoding doesn't work when the username and/or password has more complicated examples. It is impossible for URI to tell in all cases what part is what - e.g. what if my password was foo@lookslikehost.com:1234&lookslikeparam=?

As a less contrived example:

    public static void main(String[] args) throws Exception {
        String user = "myname";
        String pass = "mypass";
        final String str = "htytp://"+user+":"+pass+"@myhost.com";
        final URI u3 = new URI(null, str, null);
        System.out.println("uri="+u3+"; ascii="+u3.toASCIIString());
        System.out.println("userInfo="+u3.getUserInfo() + "; raw="+u3.getRawUserInfo());
        System.out.println("host="+u3.getHost());
        
        String user2 = "myname@mydomain.com";
        String pass2 = "mypass";
        final String str2 = "http://"+user2+":"+pass2+"@myhost.com";
        final URI u3b = new URI(null, str2, null);
        System.out.println("uri="+u3b+"; ascii="+u3b.toASCIIString());
        System.out.println("userInfo="+u3b.getUserInfo() + "; raw="+u3b.getRawUserInfo());
        System.out.println("host="+u3b.getHost());
    }

It doesn't get the username + password correct in the second parsed uri. It gives the output:

uri=htytp://myname:mypass@myhost.com; ascii=htytp://myname:mypass@myhost.com
userInfo=myname:mypass; raw=myname:mypass
host=myhost.com
uri=http://myname@mydomain.com:mypass@myhost.com; ascii=http://myname@mydomain.com:mypass@myhost.com
userInfo=null; raw=null
host=null

[aledsage]

@neykov I like your idea of something like:
$brooklyn:urlBuilder("http://host/path.to.jar").userInfo(external("artifactory.credentials")).build().

Or perhaps (IMO) in more "idiomatic yaml":

$brooklyn:urlBuilder:
  url: "http://host/path.to.jar"
  user: $brooklyn:external("creds", "artifactory.username")
  password: $brooklyn:external("creds", "artifactory.password")

I'd prefer the yaml we write to look like configuration rather than code (even if it is turned into method calls under the covers).

@neykov Do you want to capture this discussion in an Apache Brooklyn jira "improvement"?

[geomacy]

@aledsage, yes my comment above just meant that on reflection it's not invalid to use URI() to escape illegal characters in that way, but it still doesn't give us all we need; +1 to your suggestion about the more "idiomatic yaml", I like that syntax.

[neykov]

https://issues.apache.org/jira/browse/BROOKLYN-437
https://issues.apache.org/jira/browse/BROOKLYN-438