GenericUrl converts plus sign into space within URI path component

[vasiliy-bout]

This is a simple junit test which fails but should work according to https://tools.ietf.org/html/rfc3986#section-3.3 because + sign is allowed to be used within a path component and has no special meaning when used within a path component:

    @Test
    public void testGenericUrl() throws URISyntaxException {
        final URI uri = new URI("http://foo.bar/path+with+plus");

        final GenericUrl genericUrl = new GenericUrl(uri);

        Assert.assertEquals("/path+with+plus", genericUrl.getRawPath());
        Assert.assertEquals(ImmutableList.of("", "path+with+plus"), genericUrl.getPathParts());
    }

This bug is related to #255 but shows different behaviour. We are using google-http-client library version 1.23.0:

        <dependency>
            <groupId>com.google.http-client</groupId>
            <artifactId>google-http-client</artifactId>
            <version>1.23.0</version>
        </dependency>

From the first glance on sources, it looks like there is a bug inside toPathParts methods implementation on line 550: https://github.com/google/google-http-java-client/blob/110b3ab6fa7838dbe9926038427c712db9ed8aae/google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java#L545-L553

There a call is made to CharEscapers.decodeUri which is invalid, because JavaDoc of this method explicitly states that it must not be used for path segments (see the second paragraph): https://github.com/google/google-http-java-client/blob/110b3ab6fa7838dbe9926038427c712db9ed8aae/google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java#L78-L90

[mattwhisenhunt]

You are exactly right about how and where this is broken.

I'm marking that as wont-fix because the main purpose of this library is to connect to the 100+ Google APIs and other GenericURL unit tests seem to suggest that at least some of those APIs have non-conforming paths.

@vasiliy-bout are you hitting this bug while connecting to one of the Google APIs?

[vasiliy-bout]

No, we came across this bug while using the library to deal with other 3rd party HTTP APIs, so we will just ditch to a different library. I understand your reasons, but it looks a bit strange that Google HTTP APIs violates underlying standards... very much probably you also had reasons for this decision to keep violating them. I’d then suggest to at least document this non-conforming decision in the JavaDoc for the method, so that others will know about it before they came across this defect in production. What do you think?