-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Corrupting non-html content #39
Comments
+1 |
i ran into thus bug. It made me very depressed for several hours trying to figure out why I could not stream a pdf file. |
depressed is a good word to describe how I felt as well. This one is brutal to track down. |
checking the response content-type is not really of any help because it is not set yet, when this middleware does it's job. Any help on this issue is appreciated. A workaround for the time being would be to add these file types to the ignore list: |
I ran into this problem with jar files, too. I was trying to view a page which loads an applet, and some (but not all) jar files were being corrupted, with unicode replacement characters everywhere. It's an incredibly difficult bug to track down. I think the default ignore list should include all non-text file types. Instead of looking at the response content-type, why can't you look at the file "magic number" at the beginning of the stream? |
This is also a problem for me. Kind of a blocker. Been wasting 2 days trying to figure out the reason. Any workarounds? My URL's don't always contain the extension so |
The problem is not as much the missing of the content-type, the encoding gets mixed up, leaving all binary file downloads (which are not in exception list) corrupt. What should happen, I propose, is that if no The vanilla occurrence gives the following response:
With the middleware:
and the data is corrupted in the latter. In essence, if no |
Hey, Pieter, does this still happens in v0.5.0? The new algorithm is a
|
Yes, just started a blank project just to test. var express = require('express');
var app = express();
var PDFDocument = require('pdfkit');
var compression = require('compression');
var flash = require('express-flash');
app.use(compression());
app.use(flash());
app.use(require('connect-livereload')());
var router = express.Router();
router.get('/',function (req, res) {
var doc = new PDFDocument();
res.type('application/pdf');
res.status(200);
doc.pipe(res); // TODO why socket??
//doc.pipe(fs.createWriteStream('file.pdf'));
doc.info.Title = 'My First PDF';
doc.info.Author = 'Petrus';
doc.info.Subject = 'My subject';
doc.text('Hello World');
doc.end();
});
app.use('/', router);
var server = app.listen(3000, function () {
var host = server.address().address;
var port = server.address().port;
console.log('Example app listening at http://%s:%s', host, port);
}); the same occurs when calling |
Hey I eventually got the time to look further into this problem, and I made a draft version on the branch binary-files. |
@andi-neck Thanks. Just tested your branch. With my example above, which pipes data to the response, it doesn't work. However the response headers are correct now, but no data is sent. With my revised code to send a pdf file: var express = require('express');
var app = express();
var PDFDocument = require('pdfkit');
var compression = require('compression');
var flash = require('express-flash');
app.use(compression());
app.use(flash());
app.use(require('connect-livereload')());
var router = express.Router();
router.get('/',function (req, res) {
/* var doc = new PDFDocument();
res.type('application/pdf');
res.status(200);
doc.pipe(res); // TODO why socket??
//doc.pipe(fs.createWriteStream('file.pdf'));
doc.info.Title = 'My First PDF';
doc.info.Author = 'Petrus';
doc.info.Subject = 'My subject';
doc.text('Hello World');
doc.end();*/
res.sendFile('/home/pieter/AbsoluteNetwork.pdf');
});
app.use('/', router);
var server = app.listen(3000, function () {
var host = server.address().address;
var port = server.address().port;
console.log('Example app listening at http://%s:%s', host, port);
}); this works. Also using a read stream as below works with yours. Perhaps pdfkit is now the culprit, although it works perfectly without connect-livereload fs.createReadStream('/home/pieter/AbsoluteNetwork.pdf').pipe(res); |
The new version 0.5 solved the issue for static content...still wasted half a day before upgrading...dynamic content still corrupted |
Me too :( One day lost. I use dynamic content (tar), so your fix does not work. |
@gsikorski which version of |
I am using latest version (0.5.3). My pseudo code is:
|
hmm, might be a problem when piping stuff. |
Any idea on when this is going to be fixed? |
In my connect-livereload configuration, I passed the paths or extensions I want to ignore
After several months of working around this issue, this single, non-hacky change fixed it. |
I'm another person who spent about 4 hours trying to figure out why streaming out of GridFS was failing. @icompuiz Thanks for your fix. I wasn't aware of the |
The ignore option might be helpful, the problem is though that the paths are user specific, and every one would have to adjust it according to their project setup. I think we should work toward a solution that is save for binary files as well, so that they don't get corrupted. Can someone provide a PR with a test case that is corrupting binary files? Also any stream oriented solution would be welcomed, I would be happy to merge it. |
Lost several hours tracking down the issue. What I do is simply: |
So I, too, was bitten by this and spent quite some time on it. Here are the request details. Note that the URL does not contain any indication of the file type, but the "Content-Disposition" header does.
|
Hi! I should have looked at this before... Here is a full example usinge the demo from excelbuilderjs-node to build and serve an excel sheet from express. Steps to reproduce:
|
This issue drove me crazy for two days while working on zip streaming! Is there no way to block the injection of the script to html pages? I will try ignore "/api" |
@romain10009 You can use the
The parameter is a list of strings or Regex: app.use(require('connect-livereload')({
port: 35729,
ignore: ['.js', '.svg']
})); The default is: ignore: [
/\.js(\?.*)?$/, /\.css(\?.*)?$/, /\.svg(\?.*)?$/, /\.ico(\?.*)?$/, /\.woff(\?.*)?$/,
/\.png(\?.*)?$/, /\.jpg(\?.*)?$/, /\.jpeg(\?.*)?$/, /\.gif(\?.*)?$/, /\.pdf(\?.*)?$/
] If you want to ignore |
Yes sure, i can use this feature. I just dont understand what the point is
|
+1 Glad someone is documenting this one. Killed a chunk of time for me as well. |
…load' middleware; it lets to workaround a problem with malformed binary files (intesso/connect-livereload#39)
+1 |
this is screwing us over on |
Wow after HOURS of debugging chunked Transfer-Encoding I find this issue. I was trying to send a chunked response of a csv file. I tried adding The problem is that |
I was having same issue. I was using if (process.env.NODE_ENV === 'production') {
app.use(compression({}));
} |
When livereload middleware is installed I'm finding that non-html content such as generated PDFs are being corrupted. I believe this is because livereload is trying to inject the reload script snippet into the content.
I notice that the code currently checks the request accept header however wouldn't it be more appropriate to check the response content-type header before injecting the script instead?
The text was updated successfully, but these errors were encountered: