fresh is not ignoring empty http tokens

[mreinstein]

It appears that fresh has copy-pasted logic from send related to etag handling.

This commit should probably be copied in it's entirety over to fresh, right? : https://github.com/pillarjs/send/commit/aeb69c607bd01be2009c1ec822dab36151544421

It does raise the question if it might be possible to package up in a more re-usable way since there is some overlap between parts of the internals of fresh (req, res) and send's isPreconditionFailure. Or perhaps they are just different enough to warrant the duplication.

Anyway, if you agree that this commit should be copied over from send happy to send a PR.

thanks for these 2 fantastic modules by the way! ❤️ I'm working on improving koa's range support and send, fresh have both been incredibly rich sources towards better compliance with rfc7233, rfc 7232

[dougwilson]

Hi @mreinstein no problem at all! For the time being, let's just copy it over vs extract. I'll look into some kind of clean up. Please be sure to include a unit test that demos the bug so it doesn't regress again ❤️

[dougwilson]

I took a look at the code and did some tests and it seems like the token being empty doesn't affect the result, as there is a guard against empty etag and so an empty token cannot match that. But without getting what the headers you are seeing that is returning the wrong result it's hard to tell.

[mreinstein]

did some tests and it seems like the token being empty doesn't affect the result

Yeah, I confirmed that too, after writing a unit test to try to cause fresh to fail. Maybe as I was alluding to in my first comment that these 2 implementations are "different enough" to warrant the copy-pasta.

I guess that guard doesn't really hurt, right? If nothing else it makes the parseTokenList implementations 100% identical.

[mreinstein]

said differently,

parseTokenList(' "foo",,"bar" ,')  // produces [ '"foo"', '', '"bar"', '' ] seems wrong 

probably good to cause that function to not require cleanup