c4dt / lightarti-rest

Arti-wrapper to use REST over Tor
MIT License
5 stars 3 forks source link

mobile / arti-interface #28

Closed ineiti closed 3 years ago

ineiti commented 3 years ago

On the arti-rest side, the call takes a http::Request. On the mobile side I propose we start with a call with the following signature:

arti_post(
  method: string,
  url: string, 
  headers: HashMap<string, Vec<string>>, 
  body: Vec<u8>, 
  artiDic1: string, 
  artiDic2: string) -> ArtiResult

struct ArtiResult {
  // If an error is produced by arti, the length of this string will be > 0
  // in java it can be 'null', not sure about iOS
  error: string,
  // status of the response, parsed in arti-rest
  status: int,
  // the returned headers
  headers: HashMap<string, Vec<string>>,
  // the returned body
  body: Vec<u8>

This method is blocking until a result or an error is returned. For every call, a new tor-circuit will be opened using the information passed in artiDic1 and artiDic2.

Even though the signature is written in Rust, we don't use a Result return, as in fact it will be implemented in the ffi-parts and on the mobile.

ineiti commented 3 years ago

@ubamrein writes (redacted, translated, and answered by me):

tharvik commented 3 years ago
  • headers should be HashMap instead of Vec -> fixed, but one header entry can appear multiple times, and then the order matters, so it's a HashMap<string, Vec<string>>

Is having multiple times the same header-name is a use case for NotifyMe?

struct ArtiResult { // If an error is produced by arti, the length of this string will be > 0 error: string,

From what I remember of Java, we can use null to signal a missing field.

ubamrein commented 3 years ago

@ubamrein writes (redacted, translated, and answered by me):

  • why pass artiDicN strings everytime? -> because it sets up a new circuit everytime and I don't know how to cache these in a convenient way

Caching could be implemented as instance functions (always providing a reference to a rust object on the heap) instead of "static" functions, but is not really necessary :)

  • why not pass a HttpMethod? -> this first implementation is the mvp for you to be able to use it as fast as possible. We hope to being able to continue development and have a nicer interface with other HttpMethods, choice of precomputed circuits or downloading, ...

I just thought the overhead of adding one more argument (in the case of a C-style enum an integer for example) would be minimal to proceed, and would help to reduce future refactorings

  • headers should be HashMap instead of Vec -> fixed, but one header entry can appear multiple times, and then the order matters, so it's a HashMap<string, Vec<string>>

Having multiple headers appearing with the same name is not something which is used by NotifyMe, but still the overhead of doing it right is probably not too large. Not allowing this, will break most authentication apps, since they often use multiple cookies for session management

ineiti commented 3 years ago

And if I just put a method: string in there?

ubamrein commented 3 years ago

And if I just put a method: string in there?

Yeah sure, just enums sometimes give you more convenient handling. But I guess it does not matter :)

cgrigis commented 3 years ago

Here is what I have so far on the Java side (WiP):

public native HttpResponse tlsPost(String cacheDir, String url, Map<String, List<String>> headers, byte[] body);

public class HttpResponse {
    private int status;
    private String version;
    private Map<String, List<String>> headers;
    private byte[] body;

    private String data;

    public HttpResponse(int status, String version, Map<String, List<String>> headers, byte[] body) {
        this.status = status;
        this.version = version;
        this.headers = headers;
        this.body = body;
    }

    public int getStatus() {
        return status;
    }

    public String getVersion() {
        return version;
    }

    public Map<String, List<String>> getHeaders() {
        return headers;
    }

    public byte[] getBody() {
        return body;
    }
}

And on the Rust side:

pub unsafe extern "system" fn Java_org_c4dt_artiwrapper_JniApi_tlsPost(
    env: JNIEnv,
    _: JClass,
    cache_dir_j: JString,
    url_j: JString,
    headers_j: JObject,
    body_j: jbyteArray
) -> jobject {

with an implementation that uses Client.send() along with the conversion of arguments back and forth.

Example usage from Android:

        byte[] body = "key1=val1&key2=val2".getBytes();
        Map<String, List<String>> headers = new HashMap<>();
        headers.put("header-one", Collections.singletonList("hello"));
        headers.put("header-two", Arrays.asList("how", "are", "you"));
        headers.put("Content-Length", Collections.singletonList(String.valueOf(body.length)));
        headers.put("Content-Type", Collections.singletonList("application/x-www-form-urlencoded"));
        try {
            HttpResponse resp = jniApi.tlsPost(cacheDir, "https://httpbin.org/post", headers, body);
            Log.d(TAG, "Response from POST: ");
            Log.d(TAG, "   status: " + resp.getStatus());
            Log.d(TAG, "   version: " + resp.getVersion());
            Log.d(TAG, "   headers: " + resp.getHeaders());
            Log.d(TAG, "   body: " + new String(resp.getBody()));
        } catch (Exception e) {
            Log.d(TAG, "!!! Exception: " + e);
        }

I will add the sideload arguments and make the request type an argument. Could we give the artiDicX arguments a more meaningful name?

ineiti commented 3 years ago

@cgrigis - wow, very nice! I blame my 2nd CoVid-19 shot for not being as advanced as you on the iOS side ;)

Could we give the artiDicX arguments a more meaningful name?

I wonder whether we should even make it a structure. Last time I counted, there need to be 4 strings in there. So it gets a lot of arguments.

For @laurent-girod 's work, I think we have the following - but feel free to correct me...

cgrigis commented 3 years ago

Android implementation in #32 and c4dt/tor-android#2

cgrigis commented 3 years ago

Maven packages: https://search.maven.org/artifact/io.github.c4dt/artiwrapper

ineiti commented 3 years ago

iOS also done