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.

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

ETag question. About disable or not #2290

Closed
hex7c0 opened this issue Aug 8, 2014 · 12 comments
Closed

ETag question. About disable or not #2290

hex7c0 opened this issue Aug 8, 2014 · 12 comments

Comments

@hex7c0
Copy link

hex7c0 commented Aug 8, 2014

Hi,

if I disable "etag" option with app.disable('etag'); and after that send a random string like res.send('ciao'); no ETag header will display. ok

but if I send a static file (still app.disable('etag');) like res.sendFile('ciao.html'); ETag header will display. Is it correct?

@dougwilson
Copy link
Contributor

@raymondfeng can someone help this user?

@dougwilson
Copy link
Contributor

@altsang @ritch and whoever else.

@ritch
Copy link
Member

ritch commented Aug 12, 2014

For a solution that works right now you can do:

res.sendFile('ciao.html', {etag: false});

Long term there are a couple issues with app.set('etag') and send. At first glance, I don't think they support the same options. This could be changed though in a couple ways.

@dougwilson what if we moved the etag utilities into a separate module and used that to handle weak, strong and other options in both express and send?

@dougwilson
Copy link
Contributor

The etags in express use a checksum method to calculate the etags. send cannot do this, because if your file is 5GB, it would not be reasonable to calculate the md5 hash of other checksum from the contents.

@dougwilson
Copy link
Contributor

@ritch you're welcome to submit a PR over to send if you can think of a way to calculate the same kind of ETags as express does without buffering the contents of the file in memory or reading the file twice from disk.

@ritch
Copy link
Member

ritch commented Aug 12, 2014

@dougwilson do you think app.set('etag', false) should default sendFile()'s options.etag to false? Perhaps overridable by sendFile(path, {etag: true})?

...and just ignore weak and strong like we are currently?

@dougwilson
Copy link
Contributor

In express 5.0, maybe. It's a pretty breaking change.

@ritch
Copy link
Member

ritch commented Aug 12, 2014

In express 5.0, maybe. It's a pretty breaking change.

I guess the only real issue here is globally setting send to not compute etags. Maybe we can add a new setting for this? Perhaps app.set('sendFile etag', false)?

@dougwilson
Copy link
Contributor

I don't know. The setting casing doesn't really fit with the others. Anyway, if you think it's a good idea, you may want to create a new issue in the repo marked as "idea" to get input from the community.

@Fishrock123
Copy link
Contributor

Maybe just make the etag setting pass to send in a future breaking version?

@ritch
Copy link
Member

ritch commented Aug 12, 2014

Maybe just make the etag setting pass to send in a future breaking version?

#2294

@ritch ritch closed this as completed Aug 12, 2014
@hex7c0
Copy link
Author

hex7c0 commented Aug 12, 2014

thanks @ritch

hex7c0 added a commit to hex7c0/monitode that referenced this issue Aug 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants