Fix serving binary files #885
Conversation
|
Well that didn't work :/, but I am certain converting the buffer to a string for everything is breaking images. |
|
Well, I'm really at a loss as an outside contributor on this one. This change breaks all the karma preprocessors out there that expect a string as the The correct way to fix this IMO would be to pass a buffer to all preprocessors, and let them convert to a string as needed. Though the only way I can think about doing this with backwards compatibility is to check the mime type, and only convert to string if it is a text file. For now, I leave this one up to the maintainer(s) for the solution, but at least this one is tracked down. |
|
Sorry for delays @drkibitz, I'm looking into it... |
|
I'm not sure what the best solution/fix is. If I was strongly for using buffers, I would break the API. I'm however not sure buffers are the best in this case. So at this point, I'm thinking about using buffers for binary content and probably disable preprocessing of binaries. |
|
I see your point of view. Really the issue this PR is related to is about serving images correctly. If binary files skip preprocessors completely to fix this, that's totally fine. If at some point any unforeseen use case of binary preprocessors comes up, it can be a separate pipeline such as |
For binary files, we can't cache the content as a string (utf8), we need to use a binary buffer. All the preprocessors are currently implemented to deal with strings, as that makes sense for text files. If we changed preprocessors to deal with buffers then each preprocessor would have to convert the buffer to a string, do the string manipulation and then convert it back to a buffer. I don't think that's worthy as there are no binary preprocessors. This change disables preprocessing of binary files to avoid weird errors. It shows warning. If there is a reasonable use case for preprocessing binary files we can figure out something. Closes #864 Closes #885
For binary files, we can't cache the content as a string (utf8), we need to use a binary buffer. All the preprocessors are currently implemented to deal with strings, as that makes sense for text files. If we changed preprocessors to deal with buffers then each preprocessor would have to convert the buffer to a string, do the string manipulation and then convert it back to a buffer. I don't think that's worthy as there are no binary preprocessors. This change disables preprocessing of binary files to avoid weird errors. It shows warning. If there is a reasonable use case for preprocessing binary files we can figure out something. Closes #864 Closes #885
fixes #864