diegomura / jay-peg

Performant JPEG decoder
MIT License
7 stars 2 forks source link

Attempting to decode images with extra data after EOI marker fails #6

Open wyozi opened 4 months ago

wyozi commented 4 months ago

Seems like some JPEG encoders stuff extra data after EOI marker (see e.g. https://exiftool.org/forum/index.php?topic=4374.0). We bumped into this in real world usage (with react-pdf).

Would it be possible to end decoding at EOI marker and ignore rest of the data?

wyozi commented 4 months ago

This is a patch that can be used to fix this, although doesn't feel like the ideal way:

diff --git a/node_modules/jay-peg/src/index.js b/node_modules/jay-peg/src/index.js
index ba77471..d19a573 100644
--- a/node_modules/jay-peg/src/index.js
+++ b/node_modules/jay-peg/src/index.js
@@ -7,9 +7,9 @@ import DRIMarker from "./markers/dri.js";
 import EndOfImageMarker from "./markers/eoi.js";
 import EXIFMarker from "./markers/exif.js";
 import JFIFMarker from "./markers/jfif.js";
-import SOSMarker from "./markers/sos.js";
 import StartOfFrameMarker from "./markers/sof.js";
 import StartOfImageMarker from "./markers/soi.js";
+import SOSMarker from "./markers/sos.js";

 const UnkownMarker = {
   length: r.uint16be,
@@ -46,6 +46,12 @@ const Marker = new r.VersionedStruct(r.uint16be, {
   0xffe1: EXIFMarker,
 });

+Marker.process = function (stream) {
+  if (this.version === 0xffd9) {
+    stream.pos = stream.length; // finish reading
+  }
+};
+
 const JPEG = new r.Array(Marker);

 const decode = (buffer) => {
gabrielhamel commented 3 months ago

Can be something like that if you parse manually the array:

diff --git a/src/index.js b/src/index.js
index ba77471..4ec1c9f 100644
--- a/src/index.js
+++ b/src/index.js
@@ -46,10 +46,23 @@ const Marker = new r.VersionedStruct(r.uint16be, {
   0xffe1: EXIFMarker,
 });

-const JPEG = new r.Array(Marker);
-
 const decode = (buffer) => {
-  const markers = JPEG.fromBuffer(buffer);
+  let pos = 0;
+  const markers = [];
+  let marker;
+
+  do {
+    marker = Marker.fromBuffer(buffer.slice(pos));
+    markers.push(marker);
+
+    const markerLength = marker.length ?? 0;
+    pos += r.uint16be.size() + markerLength;
+
+    if (marker.name === "SOS") {
+      pos += marker.data.length;
+    }
+  } while (marker.version !== 0xffd9);
+
   return markers.map(({ version, ...rest }) => ({ type: version, ...rest }));
 };
jugheadjones10 commented 2 months ago

This is a patch that can be used to fix this, although doesn't feel like the ideal way:

diff --git a/node_modules/jay-peg/src/index.js b/node_modules/jay-peg/src/index.js
index ba77471..d19a573 100644
--- a/node_modules/jay-peg/src/index.js
+++ b/node_modules/jay-peg/src/index.js
@@ -7,9 +7,9 @@ import DRIMarker from "./markers/dri.js";
 import EndOfImageMarker from "./markers/eoi.js";
 import EXIFMarker from "./markers/exif.js";
 import JFIFMarker from "./markers/jfif.js";
-import SOSMarker from "./markers/sos.js";
 import StartOfFrameMarker from "./markers/sof.js";
 import StartOfImageMarker from "./markers/soi.js";
+import SOSMarker from "./markers/sos.js";

 const UnkownMarker = {
   length: r.uint16be,
@@ -46,6 +46,12 @@ const Marker = new r.VersionedStruct(r.uint16be, {
   0xffe1: EXIFMarker,
 });

+Marker.process = function (stream) {
+  if (this.version === 0xffd9) {
+    stream.pos = stream.length; // finish reading
+  }
+};
+
 const JPEG = new r.Array(Marker);

 const decode = (buffer) => {

I can confirm that I was facing the same issue. I had an image that just didn't load in react-pdf, but this change fixed the problem.