Strict content type checking for REST and RPC endpoints

Enable strict content type checks for REST and RPC endpoints and enable it by default.

Allowed content types are: 
JSON -> application/json, text/x-json, application/javascript, application/x-javascript, text/javascript, text/ecmascript
XML -> application/xml, text/xml
Atom -> application/xml+atom

The default output content type for JSON is switched from application/json to application/javascript to enable more convenient browser tests as application/javascript doesnt automatically trigger a download on many browsers.

application/x-www-form-urlencoded is now strictly forbidden and will result in an BAD_REQUEST. This is necessary to avoid prevent confusion around OAuth body signing. Servlets will automatically decode the body content into parameters if this content type is set and the body content is not available for the API once this happens. If the body was a JSON AppData update for instance it is lost. There is incompatability here whith Shindig PHP which allows the body through in this case and includes it in the OAuth message verification where Java Shindig simply cannot. A forthcoming patch will address the OAuth compatability issue

Containers can control whether they allow unknown content types to pass through the check

[awiner, 2009-03-13 23:11:04]

http://codereview.appspot.com/28042/diff/1/7
File java/common/src/main/java/org/apache/shindig/protocol/ApiServlet.java
(right):

http://codereview.appspot.com/28042/diff/1/7#newcode56
Line 56: 
typo?  application/javascript?

http://codereview.appspot.com/28042/diff/1/7#newcode63
Line 63: 
FORMAT_TO_PREFERRED_CONTENT_TYPE maybe better

http://codereview.appspot.com/28042/diff/1/7#newcode67
Line 67: 
some Javadoc for what this constant means?

http://codereview.appspot.com/28042/diff/1/7#newcode170
Line 170: 
Some javadoc, e.g.
/**
  * Checks if the request uses a valid content types for POST and PUT.
  */

[louiscryan, 2009-03-13 23:23:54]

Wrong patch last time, sorry

[beaton, 2009-03-14 00:17:18]

I don't know enough about the social-api side of the tree to be particularly
useful on this, did read it over though.

http://codereview.appspot.com/28042/diff/19/1020
File java/common/src/main/java/org/apache/shindig/protocol/ApiServlet.java
(right):

http://codereview.appspot.com/28042/diff/19/1020#newcode85
Line 85: public static final String CONTENT_TYPE = "CONTENT_TYPE";
I can't find this used anywhere.

http://codereview.appspot.com/28042/diff/19/1020#newcode197
Line 197: return contentType.trim().toLowerCase();
Might be nice to have a standard utility function for parsing these somewhere. 
(There is another version of this function in HttpResponse.java,
getAndUpdateEncoding.  That's a slightly more robust parser, but not by much.)

http://codereview.appspot.com/28042/diff/19/1014
File
java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java
(right):

http://codereview.appspot.com/28042/diff/19/1014#newcode128
Line 128: bodyReader = servletRequest.getReader();
This code will need changing when we add body signature checking.  I think the
strategy might end up being:

- filter calls getInputStream, reads byte array, checks signature
- filter replaces input-stream and/or reader with the byte array

http://codereview.appspot.com/28042/diff/19/1006
File
java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java
(right):

http://codereview.appspot.com/28042/diff/19/1006#newcode131
Line 131: //req.setContentType()
can delete this

[louiscryan, 2009-03-24 21:19:20]

Made json output application/json

http://codereview.appspot.com/28042/diff/19/1020
File java/common/src/main/java/org/apache/shindig/protocol/ApiServlet.java
(right):

http://codereview.appspot.com/28042/diff/19/1020#newcode85
Line 85: public static final String CONTENT_TYPE = "CONTENT_TYPE";
On 2009/03/14 00:17:18, beaton wrote:
> I can't find this used anywhere.

Removed.

http://codereview.appspot.com/28042/diff/19/1020#newcode197
Line 197: return contentType.trim().toLowerCase();
On 2009/03/14 00:17:18, beaton wrote:
> Might be nice to have a standard utility function for parsing these somewhere.

> (There is another version of this function in HttpResponse.java,
> getAndUpdateEncoding.  That's a slightly more robust parser, but not by much.)

Leaving for a later refactoring. Added comment

http://codereview.appspot.com/28042/diff/19/1014
File
java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java
(right):

http://codereview.appspot.com/28042/diff/19/1014#newcode128
Line 128: bodyReader = servletRequest.getReader();
On 2009/03/14 00:17:18, beaton wrote:
> This code will need changing when we add body signature checking.  I think the
> strategy might end up being:
> 
> - filter calls getInputStream, reads byte array, checks signature
> - filter replaces input-stream and/or reader with the byte array

Servlet wrapper now takes care of this.

http://codereview.appspot.com/28042/diff/19/1006
File
java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/AbstractLargeRestfulTests.java
(right):

http://codereview.appspot.com/28042/diff/19/1006#newcode131
Line 131: //req.setContentType()
On 2009/03/14 00:17:18, beaton wrote:
> can delete this

Done.

[louiscryan, 2009-03-24 21:19:50]

Updated patch and for bitrot

[awiner, 2009-03-24 21:32:10]

Looks good.  One recommended refactoring to keep ApiServlet small(er).

http://codereview.appspot.com/28042/diff/1049/1065
File java/common/src/main/java/org/apache/shindig/protocol/ApiServlet.java
(right):

http://codereview.appspot.com/28042/diff/1049/1065#newcode200
Line 200: String contentType) throws InvalidContentTypeException {
checkContentTypes() - and all the constants - should be in a separate class,
injected into ApiServlet.  Or just into the subclasses where it's used. 
I'd probably name it ContentTypes.

http://codereview.appspot.com/28042/diff/1049/1061
File
java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java
(right):

http://codereview.appspot.com/28042/diff/1049/1061#newcode75
Line 75: * @return An object whos toString method will return json
whos/whose

http://codereview.appspot.com/28042/diff/1049/1061#newcode145
Line 145: return value instanceof String ? Boolean.valueOf((String)value) :
Boolean.TRUE.equals(value);
space after )

[louiscryan, 2009-03-25 00:33:11]

Updated as per comments. Committing

http://codereview.appspot.com/28042/diff/1049/1065
File java/common/src/main/java/org/apache/shindig/protocol/ApiServlet.java
(right):

http://codereview.appspot.com/28042/diff/1049/1065#newcode200
Line 200: String contentType) throws InvalidContentTypeException {
On 2009/03/24 21:32:11, awiner wrote:
> checkContentTypes() - and all the constants - should be in a separate class,
> injected into ApiServlet.  Or just into the subclasses where it's used. 
> I'd probably name it ContentTypes.

Done.

http://codereview.appspot.com/28042/diff/1049/1061
File
java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java
(right):

http://codereview.appspot.com/28042/diff/1049/1061#newcode75
Line 75: * @return An object whos toString method will return json
On 2009/03/24 21:32:11, awiner wrote:
> whos/whose

Done.

http://codereview.appspot.com/28042/diff/1049/1061#newcode145
Line 145: return value instanceof String ? Boolean.valueOf((String)value) :
Boolean.TRUE.equals(value);
On 2009/03/24 21:32:11, awiner wrote:
> space after )

Done.

[Rohit Rai, 2009-06-06 07:25:10]

Has someone analyzed the side effects of this change on the existing container
implmentations. I am having a problem with the sample conainer bundled with
Shindig which uses application/x-www-form-urlencoded for setstate and
setevilness calls.

I am new to Shindig and may be wrong. Will be good if someone experienced
verifies this -
Line #117 in samplecontainer.js makes a call to the server -  

        sendRequestToServer('setstate', 'POST',
            gadgets.io.encodeValues({
                "fileurl" : stateFileUrl
            }),
            opt_callback);

Which should be handled by SampleContainerHandler service, but gets blocked by
ApiServlet and throws an error saying "Cannot use disallowed Content-Type
application/x-www-form-urlencoded"

These are the files effected -

samplecontainer.html
samplecontainer.js
SampleContainerHandler.java


On 2009/03/25 00:33:11, louiscryan wrote:
> Updated as per comments. Committing
> 
> http://codereview.appspot.com/28042/diff/1049/1065
> File java/common/src/main/java/org/apache/shindig/protocol/ApiServlet.java
> (right):
> 
> http://codereview.appspot.com/28042/diff/1049/1065#newcode200
> Line 200: String contentType) throws InvalidContentTypeException {
> On 2009/03/24 21:32:11, awiner wrote:
> > checkContentTypes() - and all the constants - should be in a separate class,
> > injected into ApiServlet.  Or just into the subclasses where it's used. 
> > I'd probably name it ContentTypes.
> 
> Done.
> 
> http://codereview.appspot.com/28042/diff/1049/1061
> File
>
java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java
> (right):
> 
> http://codereview.appspot.com/28042/diff/1049/1061#newcode75
> Line 75: * @return An object whos toString method will return json
> On 2009/03/24 21:32:11, awiner wrote:
> > whos/whose
> 
> Done.
> 
> http://codereview.appspot.com/28042/diff/1049/1061#newcode145
> Line 145: return value instanceof String ? Boolean.valueOf((String)value) :
> Boolean.TRUE.equals(value);
> On 2009/03/24 21:32:11, awiner wrote:
> > space after )
> 
> Done.