Handle Bad header in proxy

In our instance we see some random headers that are not complaint with RFC 822 section 3.1.2, which then result in illegal argument exception in our HttpServletResponse.

Added code to ignore those exception in proxy (just log).

[gagan.goku, 2010-07-27 05:36:14]

lgtm

http://codereview.appspot.com/1867046/diff/1/2
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
(right):

http://codereview.appspot.com/1867046/diff/1/2#newcode146
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:146:
// Skip illegal header
This is fine for now but probably the right way of fixing this issue everywhere
is to use UriUtils.isValidHeaderName and UriUtils.isValidHeaderValue.
Though UriUtils.isValidHeaderValue is not yet implemented, it might make sense
to do that. The rfc is clear on what values are acceptable for header names and
values.
Also, see http://codereview.appspot.com/1855044/diff/45001/46005 for another way
of refactoring ProxyHandler.

[zhoresh, 2010-07-27 20:09:03]

On 2010/07/27 05:36:14, gagan.goku wrote:
> lgtm
> 
> http://codereview.appspot.com/1867046/diff/1/2
> File
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> (right):
> 
> http://codereview.appspot.com/1867046/diff/1/2#newcode146
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:146:
> // Skip illegal header
> This is fine for now but probably the right way of fixing this issue
everywhere
> is to use UriUtils.isValidHeaderName and UriUtils.isValidHeaderValue.
> Though UriUtils.isValidHeaderValue is not yet implemented, it might make sense
> to do that. The rfc is clear on what values are acceptable for header names
and
> values.
> Also, see http://codereview.appspot.com/1855044/diff/45001/46005 for another
way
> of refactoring ProxyHandler.

Since the HttpResponse already have the logic for valid header I don't see the
real need to do it twice in this case.
Just handle the exception that is rasied for bad header.

Submitted (r979836)

[gagan.goku, 2010-07-27 20:44:52]

On 2010/07/27 20:09:03, zhoresh wrote:
> On 2010/07/27 05:36:14, gagan.goku wrote:
> > lgtm
> > 
> > http://codereview.appspot.com/1867046/diff/1/2
> > File
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> > (right):
> > 
> > http://codereview.appspot.com/1867046/diff/1/2#newcode146
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:146:
> > // Skip illegal header
> > This is fine for now but probably the right way of fixing this issue
> everywhere
> > is to use UriUtils.isValidHeaderName and UriUtils.isValidHeaderValue.
> > Though UriUtils.isValidHeaderValue is not yet implemented, it might make
sense
> > to do that. The rfc is clear on what values are acceptable for header names
> and
> > values.
> > Also, see http://codereview.appspot.com/1855044/diff/45001/46005 for another
> way
> > of refactoring ProxyHandler.
> 
> Since the HttpResponse already have the logic for valid header I don't see the
> real need to do it twice in this case.
> Just handle the exception that is rasied for bad header.
> 
> Submitted (r979836)

There is no logic in HttpResponse for checking if the header is a valid header
or not.
Also, there are no guarantees that this exception will be thrown by any other
servlet engine (jetty's Response for example does not seem to throw an
IllegalArgumentException), which is why i was saying that a better way would be
to implement the rfc compliant isValidHeaderName and isValidHeaderValue and use
these instead.
Lets try that when we revisit http headers :)

[gagan.goku, 2010-08-09 14:01:53]

Please close this issue as it has been submitted.

Thanks
Gagan