Comments (10)
PR welcome, but IMO .writeHead()
is a terrible api that people should avoid.
from compression.
Also, @wrr if you are going to make a PR, please create a new test that fails, rather than modifying an existing test.
from compression.
@wrr Any middleware cannot process things that further middleware does.
Your breaking example will always fail. (not the repurposed the unit test)
Furthermore, I have seen the use of writeHead
discouraged in mainstream use by node's maintainers, and you should probably avoid it.
from compression.
writeHead
is a part of a stable node API, it is not deprecated and middleware should not break it. writeHead
isn't also discouraged in the official node documentation. If compression
does not to support writeHead
, it IMO should explicitly fail (with an assertion in the writeHead
method). But compression
tries to implement writeHead
, the implementation is broken and your argument is that it is fine because writeHead
API is ugly?
from compression.
Oh. I'm wrong. We actually overwrite res.writeHead
so that using it after compression works properly. That means that your unit test should fail (because we don't handle that). Your example should also fail.
We only call node's writeHead after everything else, is an issue?
https://github.com/expressjs/compression/blob/master/index.js#L141-L156
As far as I can tell, we aren't breaking it.
from compression.
In my example, if you remove compression middleware, the example works fine, if you add the middleware, it returns invalid response. This means that the middleware does not support writeHead
with non empty headers argument and breaks code that uses this API. Returning invalid responses is not the same as failing. If compressions failed (with an exception), it would be easy to detect that returning headers in writeHead is not supported.
from compression.
Of course res.writeHead(200, {'Content-Length': 3})
is going to cause an invalid response, what do you expect?
from compression.
My expectations are the same as with res.setHeader('Content-Length', 3)
which does not cause an invalid response. Compression middleware is altering length of content, and it should handle changing Content-Length
headers (or replacing them with chunked encoding) in such a way that returned response remains valid. It does so correctly when headers are passed with setHeader()
, it doesn't do it when headers are passed with writeHead()
.
Are you arguing that there is no bug? My impression was that @jonathanong acknowledged the bug, but pointed that he isn't in favor of supporting 'writeHead` API.
from compression.
I'm going with: I do not understand this enough and someone else will probably have to fix it.
Apologies.
from compression.
This thread is soo annoying. It's fixed now, you're welcome.
from compression.
Related Issues (20)
- Setting Vary header although caching is disabled HOT 1
- "drain" event listener leak when using res.once("drain"); can't use res.removeListener("drain") HOT 3
- compresssion doesn't work ,the vue.txt is 2m HOT 2
- Content-Type: application/json; charset=utf-8 No effect HOT 2
- Question: Why this middleware HOT 2
- Corrupted compressed .js-files for Mac OS / Safari -clients HOT 11
- Is compression working when node server is running on a container? HOT 2
- middleware fails when the request has more than 1 values for accept-encoding header HOT 2
- Is compression result cached? HOT 1
- change Transfer-encoding HOT 1
- Why does the data size increase after compression HOT 1
- Force size to be a minimum... HOT 2
- Chunked encoding is broken after using this middleware HOT 1
- Using a current debug version HOT 1
- Deflate backwards HOT 7
- Compression instrumentation (before/after compression hooks) HOT 2
- Angular Not Compressing? HOT 2
- compression not working json payload HOT 6
- Crash when compressing characters like ū HOT 1
- Express returns a non-compliant HTTP/206 response when gzip is enabled HOT 9
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from compression.