Encode url parameters

[bazzargh]

BaseDataSource did not properly encode url parameters, meaning that users could
not log in if their password contained illegal characters. The error can be
reproduced by setting the test user password to ';/?:@&=+$,' (a bunch of illegal
characters for query parameters).

DriverTest then failed because it did not encode the parameters either.

[codecov-io]

Codecov Report

Merging #1201 into master will increase coverage by 0.37%.
The diff coverage is 68.75%.

@@             Coverage Diff              @@
##             master    #1201      +/-   ##
============================================
+ Coverage     68.67%   69.05%   +0.37%     
- Complexity     3825     3946     +121     
============================================
  Files           172      173       +1     
  Lines         15991    16528     +537     
  Branches       2603     2773     +170     
============================================
+ Hits          10982    11413     +431     
- Misses         3776     3843      +67     
- Partials       1233     1272      +39

[davecramer]

@bazzargh so it fails on Java 6 due to the fact that it java 6 does not have constructor AssertionError(java.lang.String,java.io.UnsupportedEncodingException)

Also I don't actually see a test for the user password mentioned above ?

[bazzargh]

I didn't add the test because the existing tests don't have any tests like that at all - they rely on the passwords set externally for the test and postgres users. However, I tried to add one and discovered this whole thing relies on a second oddity. The password only appears in the URL when the datasource is part of a JNDI test. BaseDataSource.getReference creates reference properties for username/password as well as other properties; when unpacking them again in setFromReference, it sets the 'special' properties correctly but then adds all reference properties back into the ds's properties object, including those that had already been handled specially.

I think I should patch both issues, the encoding issue is still a problem if any other properties include special characters, like APPLICATION_NAME. But the 'special' set of properties shouldn't leak back into the standard properties too, so I'll fix that.

[davecramer]

@bazzargh
Yes the data source code handling of properties is in need of some attention.
Is it even possible to use these special characters in APPLICATION_NAME ?

[bazzargh]

@davecramer yes, you can set that with special characters - and also the database name, which can be anything you like. I've fixed up a couple of tests to exercise this

[vlsi]

I think proper encoding/decoding makes sense.

I think we must test that otherwise we might end up in case the code fails to work for both encoded and plain passwords.

I think it might be just enough if we use special symbols for Travis PostgreSQL password. @bazzargh , what do you think?

[bazzargh]

@vlsi I think it would be better to set the application name to include special characters - particularly something like a % with non-hexadecimal after it, as that trips up URLDecoder if it's not been encoded.
The reason for using application name is that after these patches the password is no longer included in the urls generated by the datasource but other properties like application name are. But, as a double check - the tests need two passwords, one for the 'postgres' user and one for 'test', it wouldn't hurt to make one of those use special characters and leave one plain.
It may be unnecessary though, since the tests do actually cover this now. The 'bad' url generated by the datasource was eventually parsed by Driver.parseURL, where it was split at the wrong places and so the wrong password got sent to the backend. I added an explicit test that what BaseDataSource produces can be consumed by Driver in PGPropertyTest.java.

[davecramer]

@bazzargh +1 on APPLICATION_NAME
I still want to see special chars in the password, just in case pg doesn't allow them

[bazzargh]

@davecramer the reason this came up is because our DBA's create accounts with strong passwords, and we discovered that while we could use the password from the command line client, our java apps couldn't connect. The workaround was to generate a new password, but I dug in to why it had happened. During testing I was using ';/?:@&=+$,' as my password, as described above, so I'm anecdataly confirming this works in pg but not jdbc, you may want to set your tests up that way as another datapoint.

I only tested with url-special characters; I've no idea if there's any further restrictions on the password character set, eg do emoji work? But that's beyond the scope of this bug.