Update svgfilter to 0.3.1 and include emitted 'etagFragment' events i…

…n the ETag for the produced image so conditional GET works correctly with external JavaScripts.
papandreou · Aug 2, 2013 · 5a6dd93 · 5a6dd93
1 parent 983707f
commit 5a6dd93
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 5 deletions.
diff --git a/lib/processImage.js b/lib/processImage.js
@@ -35,17 +35,19 @@ module.exports = function (options) {
                         res.setHeader('Content-Type', targetContentType);
                     }
                     res.removeHeader('Content-Length');
-                    var oldETag = res.getHeader('ETag');
+                    var oldETag = res.getHeader('ETag'),
+                        newETag;
                     if (oldETag) {
-                        var newETag = '"' + oldETag.replace(/^"|"$/g, '') + '-processimage"';
+                        newETag = '"' + oldETag.replace(/^"|"$/g, '') + '-processimage"';
                         res.setHeader('ETag', newETag);
 
                         if (ifNoneMatch && ifNoneMatch.indexOf(newETag) !== -1) {
                             return res.send(304);
                         }
                     }
 
-                    var seenData = false,
+                    var etagFragments = [],
+                        seenData = false,
                         hasEnded = false;
 
                     function sendErrorResponse(err) {
@@ -66,6 +68,9 @@ module.exports = function (options) {
                         if (i < filters.length - 1) {
                             filters[i].pipe(filters[i + 1]);
                         }
+                        filters[i].on('etagFragment', function (etagFragment) {
+                            etagFragments.push(etagFragment);
+                        });
                         filters[i].on('error', sendErrorResponse);
                     }
                     res.pipe(filters[0]);
@@ -74,6 +79,17 @@ module.exports = function (options) {
                     // There are plans to fix this as part of the streams2 effort: https://github.com/joyent/node/pull/2524
                     // filters[filters.length - 1].pipe(res);
                     filters[filters.length - 1].on('data', function (chunk) {
+                        if (!seenData && oldETag && etagFragments.length > 0) {
+                            // Now that we know the additional ETag fragments there's a chance that this is still a 304.
+                            etagFragments.sort();
+                            newETag = '"' + oldETag.replace(/^"|"$/g, '') + '-' + etagFragments.join('-') + '-processimage"';
+                            res.setHeader('ETag', newETag);
+
+                            if (ifNoneMatch && ifNoneMatch.indexOf(newETag) !== -1) {
+                                hasEnded = true;
+                                return res.send(304);
+                            }
+                        }
                         seenData = true;
                         if (!hasEnded) {
                             res.write(chunk);

diff --git a/package.json b/package.json
@@ -17,7 +17,7 @@
         "pngquant": "=0.0.3"
     },
     "optionalDependencies": {
-        "svgfilter": "=0.3.0"
+        "svgfilter": "=0.3.1"
     },
     "devDependencies": {
         "expect.js": "=0.2.0",

diff --git a/test/processImage.js b/test/processImage.js
@@ -131,9 +131,14 @@ describe('test server', function () {
         request({url: baseUrl + '/dialog-information.svg?svgfilter=--runScript=addBogusElement.js,--bogusElementId=theBogusElementId'}, passError(done, function (response, svgText) {
             expect(response.statusCode).to.equal(200);
             expect(response.headers['content-type']).to.equal('image/svg+xml');
+            expect(response.headers.etag).to.be.ok();
+            expect(response.headers.etag).to.match(/^"\d+-\d+-697ebc4fd42e6b09794a5d60968435a7-processimage"$/);
             expect(svgText).to.match(/<svg/);
             expect(svgText).to.match(/id="theBogusElementId"/);
-            done();
+            request({url: baseUrl + '/dialog-information.svg?svgfilter=--runScript=addBogusElement.js,--bogusElementId=theBogusElementId', headers: {'if-none-match': response.headers.etag}}, passError(done, function (response) {
+                expect(response.statusCode).to.equal(304);
+                done();
+            }));
         }));
     });