Skip to content

Conversation

@Stebalien
Copy link
Member

@Stebalien Stebalien commented Jan 24, 2019

MOVED FROM: #3931

@jes wrote:

See on https://discuss.ipfs.io/t/writeable-http-gateways/210

Previously, a PUT to a filename that already exists would return the hash
of the newly-inserted file instead of the hash of the new tree. This is not
useful, and it makes much more sense for the API to be consistent.

Furthermore, anybody who knows about the bug is already DELETE-ing the filename
before PUT-ing the new content, so the fix doesn't trample over that behaviour
either.

With this fix, every conceivable usage of the API is either unaffected or
improved.

It could do with a unit test.


All I did was remove the special-case for the "file already exists" case, and it works correctly. It's easier to see this if you view git diff -w.

jes and others added 8 commits January 24, 2019 07:11
See on https://discuss.ipfs.io/t/writeable-http-gateways/210

Previously, a PUT to a filename that already exists would return the hash
of the newly-inserted file instead of the hash of the new tree. This is not
useful, and it makes much more sense for the API to be consistent.

Furthermore, anybody who knows about the bug is already DELETE-ing the filename
before PUT-ing the new content, so the fix doesn't trample over that behaviour
either.

With this fix, every conceivable usage of the API is either unaffected or
improved.

It could do with a unit test.

License: MIT
Signed-off-by: James Stanley <[email protected]>
This fixes the unit test but we should perhaps clarify what the intended
behaviour is when PUT'ing to an empty filename.

License: MIT
Signed-off-by: James Stanley <[email protected]>
License: MIT
Signed-off-by: James Stanley <[email protected]>
License: MIT
Signed-off-by: James Stanley <[email protected]>
License: MIT
Signed-off-by: James Stanley <[email protected]>
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien Stebalien requested a review from Kubuxu as a code owner January 24, 2019 15:31
@ghost ghost assigned Stebalien Jan 24, 2019
@ghost ghost added the status/in-progress In progress label Jan 24, 2019
@Stebalien Stebalien changed the title MOVED FROM: https://github.com/ipfs/go-ipfs/pull/3931 Fix the PUT-existing bug in writable gateway API Jan 24, 2019
@Stebalien
Copy link
Member Author

This was previously reviewed by both @whyrusleeping and @Kubuxu. I believe I've addressed their remaining comments (#3931 (review)).

@Stebalien
Copy link
Member Author

This looks like something we'll need to fix:

Tests / linuxSharness / 40 - We can HTTP GET file just replaced – sharnessLinux.t0111-gateway-writeable

Stacktrace
URL="http://localhost:$port/ipfs/$HASH/test/test.txt" && echo "GET $URL" && curl -svo outfile3 "$URL" 2>curl_getExisting.out && test_cmp infile3 outfile3
Standard Output
expecting success:
URL="http://localhost:$port/ipfs/$HASH/test/test.txt" &&
echo "GET $URL" &&
curl -svo outfile3 "$URL" 2>curl_getExisting.out &&
test_cmp infile3 outfile3
GET http://localhost:37288/ipfs/QmfGKo7mm41im56UKjRJFKYqJiTMhsmYcNazSYHSzGmxnF/test/test.txt
> diff -u infile3 outfile3
--- infile3 2019-01-24 15:34:02.691086012 +0000
+++ outfile3 2019-01-24 15:34:05.886928567 +0000
@@ -1 +1 @@
-8372
+24246
not ok 40 - We can HTTP GET file just replaced
Standard Error

@magik6k magik6k self-requested a review February 9, 2019 02:24
@Stebalien
Copy link
Member Author

Fixed in libp2p/go-libp2p-connmgr#50.

@Stebalien Stebalien closed this Jul 29, 2019
@hacdias hacdias deleted the pr/3931 branch May 9, 2023 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants