Bug 40387 - PFMFile.load() crashes if a font has a name longer than 16 characters and also if parsing pfm files larger than 2048 bytes
Summary: PFMFile.load() crashes if a font has a name longer than 16 characters and als...
Status: CLOSED FIXED
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: general (show other bugs)
Version: trunk
Hardware: All All
: P2 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords: ErrorMessage, PatchAvailable
Depends on:
Blocks:
 
Reported: 2006-09-01 11:54 UTC by Patrick Schulz
Modified: 2012-04-01 06:29 UTC (History)
0 users



Attachments
Patch for this issue (1.74 KB, patch)
2006-09-01 17:56 UTC, Patrick Schulz
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Schulz 2006-09-01 11:54:18 UTC
I think the summary is clear enough.
To solve the issues: 
1.) increase mark parameter of inStream.
2.) set a correct buffersize when convert it to a BufferdInputStream


Here is the patch that is API compatible with older versions.

Index: PFMFile.java
===================================================================
--- PFMFile.java	(revision 439270)
+++ PFMFile.java	(working copy)
@@ -70,18 +70,37 @@
     protected Log log = LogFactory.getLog(PFMFile.class);
 
     /**
+     * Parses a PFM file.
+     * It is the same as load(inStream, 0);
+     *
+     * @param  inStream The stream from which to read the PFM file.
+     * @throws IOException In case of an I/O problem
+     */
+    public void load(InputStream inStream) throws IOException {
+        load(inStream, 0);
+    }
+
+    /**
      * Parses a PFM file
      *
      * @param  inStream The stream from which to read the PFM file.
+     * @param  bufferSize If inStream doesn't support marks and it is going to
be buffered, the size the bufferd stream must have.
+     * Will be ignored if inStream supports marks. Should equal the full size
of inStream.
      * @throws IOException In case of an I/O problem
      */
-    public void load(InputStream inStream) throws IOException {
+    public void load(InputStream inStream, int bufferSize) throws IOException {
         InputStream bufin = inStream;
         if (!bufin.markSupported()) {
-            bufin = new BufferedInputStream(bufin);
+            if (bufferSize > 0)
+                bufin = new BufferedInputStream(bufin, bufferSize);
+            else
+                bufin = new BufferedInputStream(bufin);
+
         }
         PFMInputStream in = new PFMInputStream(bufin);
-        bufin.mark(16);
+// increased buff.mark() four times to avoid errors when reading fontnames
larger than 16 characters.
+//        bufin.mark(16);
+        bufin.mark(64);
         short sh1 = in.readByte();
         short sh2 = in.readByte();
         if (sh1 == 128 && sh2 == 1) {
Comment 1 Patrick Schulz 2006-09-01 17:56:50 UTC
Created attachment 18805 [details]
Patch for this issue
Comment 2 Jeremias Maerki 2006-09-11 06:40:19 UTC
I've taken a look at your patch and the fix for the font name is clear enough.
However, the buffer parameter for the BufferedInputStream puzzles me. You add a
new method signature with a buffer parameter but you didn't also make a change
in PFMReader so that the official command-line tool profits from the change. It
looks more like you added this so you can work around the problem in your
environment. I'd prefer a more general fix if possible. So, should we simply
hard-code the buffer size to 16KB or something like that to avoid the seemingly
rare problem you're experiencing?
Comment 3 Jeremias Maerki 2006-11-13 09:45:43 UTC
I finally decided not to apply your patch and to use a more radical approach
(buffering the whole file in memory). I still had problems with some PFMs even
after applying the proposed patch.
http://svn.apache.org/viewvc?view=rev&rev=474410
Comment 4 Patrick Schulz 2006-11-14 05:18:27 UTC
Hi Jeremias, sorry for my late answer. I saw accidentally that mails from
bugzilla have been filtered as junkmail, so I didn't know about your last comments.

For the rest, this was my first experience with projects like this, so please be
a little bit lenient toward me. But I'll keep your feedback in mind.

Regarding the buffer size - I'm not sure why, but with a larger size than 64 I
experienced some other problems (too long ago to remember exactly). I think the
error causes by the BufferdInputStrem itself, but therefore my experience with
Java is too short. But using a ByteArrayInputStream instead seems to be best
agreement.
Comment 5 Jeremias Maerki 2006-11-14 13:38:42 UTC
Thank you for your feedback, Patrick.

Hey, I'm glad for your patch even though I didn't apply it. It made me
investigate the problem more closely. Don't let this scare you away please!!!
Don't hesitate if have anything else to contribute at some point!
Comment 6 Patrick Schulz 2006-11-16 02:07:50 UTC
(In reply to comment #5)
> Thank you for your feedback, Patrick.
You are welcome.

> Hey, I'm glad for your patch even though I didn't apply it. It made me
> investigate the problem more closely. Don't let this scare you away please!!!
> Don't hesitate if have anything else to contribute at some point!

Oh no, I'm not scared. That's for sure.
May be I have something else to contribute. Support for MAC fonts ie.

Could I contact you directly by mail to arrange it with you?
Comment 7 Jeremias Maerki 2006-11-16 12:03:08 UTC
(In reply to comment #6)
> Oh no, I'm not scared. That's for sure.

That's good. 

> May be I have something else to contribute. Support for MAC fonts ie.
> 
> Could I contact you directly by mail to arrange it with you?

No, please subscribe to the fop-dev mailing list and post there if there's
anything development-specific. That way my colleagues can also help and
everything is properly archived. I may not be always and immediately available.
Thanks!
Comment 8 Glenn Adams 2012-04-01 06:29:39 UTC
batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed