sdouglass / spring-social-tumblr

Spring Social client implementation for Tumblr
Apache License 2.0
17 stars 12 forks source link

Avatar does not work: Tumblr API redirects on Avatar #4

Closed agentgt closed 12 years ago

agentgt commented 12 years ago

Oh man I found a totally PITA of a problem in that Tumblr will send a 301 for the Avatar API (head would not work at all).

A 301 is bad for two reasons. It makes the underlying HTTP library do a redirect (ie follow redirect) and even if follow redirect is off a 301 is an error for RestTemplate.

Here is an example solution. I'll add it to my fork soon.

private String tumblrAvatar(String name, AvatarSize size) {
    String uri = "http://api.tumblr.com/v2/blog/" + name + "/avatar/" + size.getDimension();
    RestTemplate rt = new RestTemplate();
    SimpleClientHttpRequestFactory f = new SimpleClientHttpRequestFactory() {
        @Override
        protected void prepareConnection(HttpURLConnection connection, String httpMethod) throws IOException {
            super.prepareConnection(connection, httpMethod);
            connection.setInstanceFollowRedirects(false);
        }

    };
    rt.setRequestFactory(f);
    rt.setErrorHandler(new DefaultResponseErrorHandler() {
        @Override
        protected boolean hasError(HttpStatus statusCode) {
            if (statusCode == HttpStatus.MOVED_PERMANENTLY) 
                return false;
            return super.hasError(statusCode);
        }

    });
    String avatar = rt.execute(uri, HttpMethod.GET, new RequestCallback() {
        @Override
        public void doWithRequest(ClientHttpRequest request) throws IOException {
            request.getHeaders().add("Host", "api.tumblr.com");
            request.getHeaders().setAccept(asList(MediaType.APPLICATION_JSON));
        }
    }, new ResponseExtractor<String>() {
        @Override
        public String extractData(ClientHttpResponse response) throws IOException {
            return response.getHeaders().getLocation().toString();
        }
    });
    return avatar;
}
sdouglass commented 12 years ago

Thanks for finding that one and figuring it out. Do you have some example blogs/URLs that trigger this issue? I only have a few blogs that I test with and I'm not seeing this problem, so I'd like to add whatever blogs you test with to my set of tests.

agentgt commented 12 years ago

Sam with out a custom rest template I have no idea how you could have gotten it to ever work.

The issue is not the "Head" request but the fact the server sends back a 301:

curl -I "http://api.tumblr.com/v2/blog/justinesther.tumblr.com/avatar/512"

HTTP/1.1 301 Found
P3P: CP="ALL ADM DEV PSAi COM OUR OTRo STP IND ONL"
Location: http://27.media.tumblr.com/avatar_fe4113dded20_512.png
Vary: Accept-Encoding
X-Tumblr-Usec: D=17379
Content-Type: application/json
Content-Length: 123
Date: Fri, 27 Apr 2012 15:34:18 GMT
Connection: close

Almost every ClientHttpRequestFactory will create a request that will do the redirect automatically for you. That is if its a 301 the RestTemplate will never every see the above data but what will get the redirected data.

Once redirected you get either a 400 (if accept type is "application/json") or a 200 but with no location header (ie NPE).

curl -I http://27.media.tumblr.com/avatar_fe4113dded20_512.png

HTTP/1.1 200 OK
x-amz-id-2: eqUOdEd+Pqvcm3yDR7K4oLL80YLcWzXj4DITnkPoLpmT1x+t1Kg05cHxQ7YYQ2gH
x-amz-request-id: E8BC144A0FEC8E6B
Last-Modified: Tue, 24 Apr 2012 20:41:14 GMT
x-amz-version-id: z1ViYgtrb7K4fLK8YVOKEhuOL2w3sHAM
ETag: "e46f7cdec401b9714f55ebcd7d9fbd6f"
Accept-Ranges: bytes
Content-Type: image/png
Content-Length: 143161
Server: AmazonS3
Cache-Control: max-age=31535944
Date: Fri, 27 Apr 2012 15:48:33 GMT
Connection: keep-alive

In other words the RestTemplate gets the above request and not the 301 and because there is no location header you will get a Null Pointer Exception.

If this really does work for you some how your ClientHttpRequestFactory is not the same has mine (mine defaults to HttpComponents through: ClientHttpRequestFactorySelector.getRequestFactory()).

protected AbstractOAuth1ApiBinding() {
    credentials = null;
    restTemplate = new RestTemplate(ClientHttpRequestFactorySelector.getRequestFactory());
    restTemplate.setMessageConverters(getMessageConverters());
    configureRestTemplate(restTemplate);
}

The only way I can see making this fail safe is using a custom rest template for this call.

sdouglass commented 12 years ago

I just stepped through the Spring code with a debugger and for me it's using HttpURLConnection. That must be the difference. I will give things a try with HttpComponents.

agentgt commented 12 years ago

Funny it shouldn't work with HttpURLConnection either hence why I had to overload the method prepareConnection:

SimpleClientHttpRequestFactory f = new SimpleClientHttpRequestFactory() {
    @Override
    protected void prepareConnection(HttpURLConnection connection, String httpMethod) throws IOException {
        super.prepareConnection(connection, httpMethod);
        connection.setInstanceFollowRedirects(false);
    }

};

The default prepareConnection does a connection.setInstanceFollowRedirects(true).

sdouglass commented 12 years ago

When I stepped through the code I noticed the Spring code already calls connection.setInstanceFollowRedirects(true) in SimpleClientHttpRequestFactory.prepareConnection() if the method is not "GET". In fact, it looked like everything from your first code snippet was already happening, so maybe that's why it's working for me with HttpURLConnection? Maybe you've got a different Spring version? I updated the pom.xml recently to use 3.1.0.RELEASE.

agentgt commented 12 years ago

Now that I look at it appears the only real issue is the HttpComponents request factory. It should be doing what the SimpleClientHttpReqeustFactory is doing (that is set it up so that it does not redirect on HEAD).

I got confused because I switched to GET in my implementation while I was trying to figure out whats wrong.

So this is looks like a bug with Spring HttpComponentsClientHttpRequestFactory. I'll go file a bug with them.