Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add option to specify available encodings #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add option to specify available encodings #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
View file
Open in desktop
Expand Up @@ -46,6 +46,14 @@ the response.
The default filter function uses the [compressible](https://www.npmjs.com/package/compressible)
module to determine if `res.getHeader('Content-Type')` is compressible.

##### available

The set of encodings to make available for responses. This is an array of
strings passed to the `encoding` function of the [accepts](https://www.npmjs.com/package/accepts)
module to determine the method of compression used.

The default `available` array is `['gzip', 'deflate', 'identity']`.

##### threshold

The byte threshold for the response body size before compression is considered
Expand Down
3 changes: 2 additions & 1 deletion index.js
View file
Open in desktop
Expand Up @@ -39,6 +39,7 @@ function compression(options) {
var opts = options || {}

var filter = opts.filter || shouldCompress
var available = opts.available || ['gzip', 'deflate', 'identity']
var threshold = typeof opts.threshold === 'string'
? bytes(opts.threshold)
: opts.threshold
Expand Down Expand Up @@ -157,7 +158,7 @@ function compression(options) {

// compression method
var accept = accepts(req)
var method = accept.encoding(['gzip', 'deflate', 'identity'])
var method = accept.encoding(available)

// negotiation failed
if (!method || method === 'identity') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong, but I think it always needs to have the identity fallback.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right
if you want disable deflate algorithm, you should use ['gzip', 'identity']

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I'm not sure if the interface should make the user include identity

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before force identity, people should check the presence of their algorithms inside this module

worst case? ciao like a new superpowerful algorithm :)
['ciao', 'identity'] as option
ab -v 4 -H "Accept-Encoding: ciao" -c 1 -n 1 127.0.0.1:3000/with_compression

LOG: header received:
HTTP/1.1 200 OK
Content-Type: text/plain
Vary: Accept-Encoding
Content-Encoding: ciao
Date: Mon, 26 Jan 2015 18:59:20 GMT
Connection: close

sent with deflate algorithm https://github.com/expressjs/compression/blob/master/index.js#L172

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hex7c0 , right, that is another case that needs to be fixed and why this PR hasn't been merged just yet :)

Expand Down
39 changes: 39 additions & 0 deletions test/compression.js
View file
Open in desktop
Expand Up @@ -544,6 +544,45 @@ describe('compression()', function(){
.end()
})
})

describe('available', function () {
it('should limit the encodings used', function(done){
var server = createServer({ available: ['gzip'], threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
})

request(server)
.get('/')
.set('Accept-Encoding', 'deflate;q=1.000, gzip;q=0.001')
.expect('Content-Encoding', 'gzip', done)
})

it('should prevent compression if no encoding available', function(done){
var server = createServer({ available: ['gzip'], threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
})

request(server)
.get('/')
.set('Accept-Encoding', 'deflate')
.expect(shouldNotHaveHeader('Content-Encoding'))
.expect(200, done)
})

it('should not prevent available encodings from being used', function(done){
var server = createServer({ available: ['deflate'], threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
})

request(server)
.get('/')
.set('Accept-Encoding', 'deflate')
.expect('Content-Encoding', 'deflate', done)
})
})
})

function createServer(opts, fn) {
Expand Down