RangerMauve / hypercore-fetch

Implementation of Fetch that uses the Hyper SDK for loading p2p content
MIT License
37 stars 13 forks source link

Test: `?noResolve` query string shouldn't break `HEAD`/`GET` file requests #78

Closed josephmturner closed 1 year ago

josephmturner commented 1 year ago

Current behavior: HEAD hyper://PUBLIC-KEY/file.txt?noResolve fails with status 500.

Expected behavior: HEAD hyper://PUBLIC-KEY/file.txt?noResolve returns the same response as HEAD hyper://PUBLIC-KEY/file.txt.

What is the expected behavior of adding ?noResolve query string to a file path URL?

RangerMauve commented 1 year ago

Hmm, I was thinking that adding ?noResolve should always trigger the "Read this as a directory" logic. IMO an error makes sense with that in mind. Is there a benefit to silently ignoring the case where you try to read a file as a directory?

josephmturner commented 1 year ago

Hmm, I was thinking that adding ?noResolve should always trigger the "Read this as a directory" logic. IMO an error makes sense with that in mind. Is there a benefit to silently ignoring the case where you try to read a file as a directory?

In hyperdrive.el, we never want to resolve to index.html (we always want a directory listing). Currently, for all GET/HEAD requests to URLs ending with "/", we append "?noResolve" before sending out the request.

It's a little bit awkward, but no big deal.

I was thinking it would be slightly less awkward to simply append "?noResolve" to all GET/HEAD requests regardless of whether they end with "/".

RangerMauve commented 1 year ago

sounds good, I will add the fix in

RangerMauve commented 1 year ago

oh wow looks like this hit a bug in the resolve path function. glad you raised the issue!