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

Allow users to publish with a generated key + Check minimum version fix #29

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -54,12 +54,9 @@ public IPFS(String host, int port, String version) {
// Check IPFS is sufficiently recent
try {
String ipfsVersion = version();
String[] parts = ipfsVersion.split("\\.");
String[] minParts = MIN_VERSION.split("\\.");
if (parts[0].compareTo(minParts[0]) < 0
|| parts[1].compareTo(minParts[1]) < 0
|| parts[2].compareTo(minParts[2]) < 0)
if (!versionIsValid(ipfsVersion)) {
throw new IllegalStateException("You need to use a more recent version of IPFS! >= " + MIN_VERSION);
}
} catch (IOException e) {
throw new RuntimeException(e);
}
@@ -303,11 +300,16 @@ public MerkleNode patch(Multihash base, String command, Optional<byte[]> data, O

public class Name {
public Map publish(Multihash hash) throws IOException {
return publish(Optional.empty(), hash);
return publish(hash, Optional.empty());
}

public Map publish(Multihash hash, Optional<String> key) throws IOException {
return retrieveMap("name/publish?arg=/ipfs/"+hash + (key.isPresent() ? "&key="+key : ""));
}

@Deprecated
public Map publish(Optional<String> id, Multihash hash) throws IOException {
return retrieveMap("name/publish?arg=" + (id.isPresent() ? id+"&arg=" : "") + "/ipfs/"+hash);
return retrieveMap("name/publish?arg=" + (id.isPresent()?id + "&arg=":"") + "/ipfs/" + hash);
}

public String resolve(Multihash hash) throws IOException {
@@ -490,6 +492,28 @@ public Object log() throws IOException {
}
}

private boolean versionIsValid(String version) {
String[] parts = version.split("\\.");
Copy link

@Kubuxu Kubuxu Jun 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use proper semver lib. IMO it is much better idea.

Copy link
Collaborator

@ianopolous ianopolous Jun 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library needs some love and updating in general I will try and devote some time asap.

Copy link
Collaborator

@ianopolous ianopolous Jun 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kubuxu I prefer to avoid bringing in binary external dependencies for simple things like this. One major benefit of that approach is that it makes it easy to cross compile this library to JS (which I do elsewhere).

Copy link
Member

@victorb victorb Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianopolous is there a reason you cross-compile this instead of using ipfs/js-ipfs-api?

Copy link
Collaborator

@ianopolous ianopolous Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @victorbjelkholm! All of our application logic is written in Java. Originally we had a parallel implementation in JS but it was so much slower to write correctly and error prone. Writing the original in Java and cross compiling lets us move much faster and with much more safety and guarantees. We do have a few interfaces where java code can call out to JS (or Java if it's not running in a browser), but we try to keep those to a minimum.

We are also extremely sensitive to importing random libraries, for security reasons. I have never built js-ipfs-api, but I would guess it brings in 100s of dependencies through npm. All of these would have to be audited in detail in our security audit.

Copy link
Member

@victorb victorb Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianopolous I see. Your guess about the npm dependencies is indeed correct. Thanks for explaining it!

Copy link
Collaborator

@ianopolous ianopolous Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@victorbjelkholm No worries! :-)

String[] minParts = MIN_VERSION.split("\\.");
int[] intParts = Arrays.stream(parts).mapToInt(Integer::parseInt).toArray();
int[] intMinParts = Arrays.stream(minParts).mapToInt(Integer::parseInt).toArray();
return isNewerThan(intParts, intMinParts);
}

private boolean isNewerThan(int[] current, int[] min) {
if (min.length == 0) {
return true;
} else if (current.length == 0) {
return false;
} else if (current[0] < min[0]) {
return false;
} else {
int[] tailCurrent = Arrays.copyOfRange(current, 1, current.length);
int[] tailMin = Arrays.copyOfRange(min, 1, min.length);
return isNewerThan(tailCurrent, tailMin);
}
}

private Map retrieveMap(String path) throws IOException {
return (Map)retrieveAndParse(path);
}