Bug 48337 - [PATCH] Batik sends invalid accept header when requesting images
Summary: [PATCH] Batik sends invalid accept header when requesting images
Status: RESOLVED FIXED
Alias: None
Product: Batik - Now in Jira
Classification: Unclassified
Component: Utilities (show other bugs)
Version: 1.7
Hardware: PC Windows XP
: P2 normal
Target Milestone: ---
Assignee: Batik Developer's Mailing list
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2009-12-03 13:57 UTC by daveray
Modified: 2009-12-09 03:52 UTC (History)
0 users



Attachments
Tested patch that includes the suggested fix. (984 bytes, patch)
2009-12-06 17:40 UTC, daveray
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description daveray 2009-12-03 13:57:12 UTC
When Batik requests an image via the <image> element, I am seeing an accept request header like this:

image/png,image/jpeg,image/jpg,image/tiff,image/tif,image/gif,[Ljava.lang.String;@f429d7;

Note the Java "string array" stuck on the end. This gives my Jersey/JAX-RS-based web service fits because it automatically calls the header invalid and returns 400 bad request.
Comment 1 daveray 2009-12-03 17:13:58 UTC
FWIW, the bug appears to be in SVGImageElementBridge.openStream() (line 409 on the trunk):

   List mimeTypes = new ArrayList
            (ImageTagRegistry.getRegistry().getRegisteredMimeTypes());
   mimeTypes.add(MimeTypeConstants.MIME_TYPES_SVG);

Note that MIME_TYPES_SVG is an *array* of strings which is added directly to the end of the list of mimeTypes. This explains the "[Ljava.lang.String;@f429d7;" that I'm seeing in the Accept header.

I think the fix is simply to replace the add with:

   mimeTypes.addAll(Arrays.asList(MimeTypeConstants.MIME_TYPES_SVG));

Dave
Comment 2 Helder Magalhães 2009-12-06 13:49:41 UTC
First of all, thanks for taking the time to report and chase the issue! :-)


(In reply to comment #1)
> FWIW, the bug appears to be in SVGImageElementBridge.openStream() (line 409 on
> the trunk):
>    List mimeTypes = new ArrayList
>             (ImageTagRegistry.getRegistry().getRegisteredMimeTypes());
>    mimeTypes.add(MimeTypeConstants.MIME_TYPES_SVG);

As a matter of curiosity, I've made a quick crawl through that piece of code and it has been untouched for a while (i.e., it's probably a bug which has been sitting there for a long time, not a recent regression AFAIK). It's even somehow curious how this has slipped through until now... ;-)


> Note that MIME_TYPES_SVG is an *array* of strings which is added directly to
> the end of the list of mimeTypes. This explains the
> "[Ljava.lang.String;@f429d7;" that I'm seeing in the Accept header.
> 
> I think the fix is simply to replace the add with:
> 
>    mimeTypes.addAll(Arrays.asList(MimeTypeConstants.MIME_TYPES_SVG));

The fix seems reasonable. Have you tested it? Are you able to submit the fix proposal in patch format instead? (Actually, I'm a lot more interested in the first question; if you need help with converting it to a patch, in order to ease review, I'll be more than willing to help you with that.)
Comment 3 daveray 2009-12-06 17:40:55 UTC
Created attachment 24672 [details]
Tested patch that includes the suggested fix.
Comment 4 daveray 2009-12-06 17:46:45 UTC
(In reply to comment #2)
> First of all, thanks for taking the time to report and chase the issue! :-)

No problem. For a while there, I thought I was going mad. :)

> (In reply to comment #1)
> > FWIW, the bug appears to be in SVGImageElementBridge.openStream() (line 409 on
> > the trunk):
> >    List mimeTypes = new ArrayList
> >             (ImageTagRegistry.getRegistry().getRegisteredMimeTypes());
> >    mimeTypes.add(MimeTypeConstants.MIME_TYPES_SVG);
> 
> As a matter of curiosity, I've made a quick crawl through that piece of code
> and it has been untouched for a while (i.e., it's probably a bug which has been
> sitting there for a long time, not a recent regression AFAIK). It's even
> somehow curious how this has slipped through until now... ;-)

It seems that nothing else is as picky as Jersey is about this header so I guess it will only affect people that are serving non-static images through Jersey.... I guess I'm in an exclusive club.

> > Note that MIME_TYPES_SVG is an *array* of strings which is added directly to
> > the end of the list of mimeTypes. This explains the
> > "[Ljava.lang.String;@f429d7;" that I'm seeing in the Accept header.
> > 
> > I think the fix is simply to replace the add with:
> > 
> >    mimeTypes.addAll(Arrays.asList(MimeTypeConstants.MIME_TYPES_SVG));
> 
> The fix seems reasonable. Have you tested it? Are you able to submit the fix
> proposal in patch format instead? (Actually, I'm a lot more interested in the
> first question; if you need help with converting it to a patch, in order to
> ease review, I'll be more than willing to help you with that.)

I've tested the fix (it works) and submitted a patch.

In case anyone else has this problem before the next Batik release, my temporary workaround is a servlet filter that scrubs the accept header before it gets to the Jersey servlet.
Comment 5 Helder Magalhães 2009-12-07 03:13:45 UTC
(In reply to comment #4)
> It seems that nothing else is as picky as Jersey is about this header so I
> guess it will only affect people that are serving non-static images through
> Jersey.... I guess I'm in an exclusive club.

:-D


> I've tested the fix (it works) and submitted a patch.

Great, thanks. :-)

Thomas, I've also reviewed the fix and it seems rather simple and reasonable. Could this be taken a look at?

In the meantime, I've added metadata to the bug in order to reflect the fact that a patch is available.
Comment 6 Thomas Deweese 2009-12-09 03:52:38 UTC
Fixed in rev 888773.
Thanks!