Restrict ipfs object patch in ro api

[rht]

add-link is local node-specific operation, so it is relatively safer. But I think it should have been removed.

• remove add-link?

[whyrusleeping]

i see no reason to restrict any of these

[rht]

I'd interpret that as "the 'ro' in ro api refers to restricting any mutation on the graph-scale, but not on the node-scale".

[rht]

consequence: anyone could reconstruct an add through a series of object patch set-data operations.

[rht]

rfcr: @lgierth @jbenet

[rht]

It'd be helpful to get the nomenclature ambiguity cleared once for all, since in its usage, 'object' could refer to the dag while at other time it is used to refer to the local dag node (e.g. here).

[rht]

An example of usage ambiguity of the term 'object': #1891 (comment).

[whyrusleeping]

I'm in favor of closing this, yes someone could 'reproduce add' through a bunch of calls, but the difference is that add pins things. object patch creates temporary objects, and is no more harmful in that respect than any of the other commands that cause the node to fetch an object from another machine.

[ghost]

object patch creates temporary objects, and is no more harmful in that respect than any of the other commands that cause the node to fetch an object from another machine

Agreed

[rht]

So technically, 'RO' refers to restricting against pinning?

[whyrusleeping]

eh, i'm not gonna give a concrete definition of what it refers to. its really just API's that we feel should be there

[rht]

Even if you're not putting a concrete definition, that's what the RO API is as it is currently. It allows unpinned dag-scale add, which is more direct than implicit add through GET. Allowing 'add' doesn't sound like what 'RO' usually refer to. I heard there is something called 'append' for the case of acl?

@(go-ipfs maintainers), right, it's your branch and your rules. I'm just making suggestion+reasons that it should not be there. Feel free to close it if you have concluded otherwise.

[jbenet]

Hm. This is tricky. Because as a user, if I expose a read only thing, I wouldn't expect that to cause data added to my disk. I understand the get problem though.

It's likely we should rename "RO" to something else. I'm not sure what. But which included reading objects from the network and add without pinning.

We could then restrict further to a true RO mode where it only hits only local storage or otherwise a mode where it fetches from network but strictly in memory only, not disk. I'm not sure this is needed yet.

I'd probably just hange the name. Anyone have any suggestions?

[Kubuxu]

Read-only API should allow for at least for what gateway itself allows so I see no reason to disallowing network fetches.

Temporary write would be really useful feature but without pinning there is non-zero risk that object will be GCed before anything can be done with it. This could be solved by temporary pin, pin working only for 5-10min (maximum value declared in code and by user, actual in the request).

Other thing that was mentioned elsewhere is that many API commands are not side-effect free (they fetch data and save it on disk) but use GET method. If this was to be fixed, GET should be instant and return data already in node and alternative HTTP method (FETCH) should be used when data should be fetched from network and saved.

[rht]

alternative HTTP method (FETCH) should be used when data should be fetched from network and saved.

AFAIU, the side effect concerned with FETCH, while it counts as fetching resource from remote node (addressed by this node (could be domain name or ipns peerid) instead of by the data's content), happens fully on the client side.

This could work if the client is/has an IpfsNode, where it requests GET to a full-ro gateway. If the gateway notifies it doesn't have the resource, then this client could proceed to FETCH it elsewhere from the network. From permission standpoint, an IpfsNode is equivalent to a full-ro gateway, except that it receives its request from the wantlist messages indirectly from the network.

If the client is not an IpfsNode, then GET should be split into a ro-no-side-effect GET and a request method that appends a hash into the gateway's wantlist.

Currently, DAG.Get does not distinguish between the data available locally, in remote IpfsNode1, or in remote IpfsNode2, or if there is a specific default node to begin a get from (e.g. a gateway). I think, at least the local option not being baked into the dagservice spec has caused many issues with the offline-only operations, where the workaround/hack is to temporarily use offline routing or resolve to key (e.g. #2201).

---

While side-effects are relevant to this PR, this PR is specific on the issue of allowing directly building a node/graph on a "RO" gateway (currently, ipfs object put is still enabled for 0.4.0 ipfsnodes!). For gx's use case, it should use RA (read-append) or RP (read-patch, but this collides with 'P' as in 'pin'), without compromising other gateway users using the RO one. Please merge this PR soon, as it still allows gx to work while reducing the size of the hole.

Temporary write would be really useful feature but without pinning there is non-zero risk that object will be GCed before anything can be done with it.

This is also a problem in local ipfs add, and has caused occassional ci stall. It appears that PinLock is used for persistent lock, and GCLock for this sync write problemPinLock should also be used for add whether pinning happens or not.

[whyrusleeping]

closing, i'm fine with how patch currently is exposed. If anyone has a really compelling argument otherwise file an issue to discuss.