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

POC protobuf communication over FFI #626

Merged
merged 1 commit into from
Feb 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,10 @@ captures

# iOS stuff.
xcuserdata

# Protobuf Swift
*.pb.swift

# Carthage built artifacts
Cartfile.resolved
Carthage
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ before_deploy:
- rustup target add aarch64-apple-ios armv7-apple-ios i386-apple-ios x86_64-apple-ios
- cargo install cargo-lipo
- brew update && (brew outdated carthage || brew upgrade carthage)
- brew install swift-protobuf
- carthage bootstrap
- carthage build --no-skip-current --verbose
- carthage archive --output $FRAMEWORK_NAME
- rm -rf Carthage
Expand Down
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@

[Full Changelog](https://github.com/mozilla/application-services/compare/v0.16.1...master)

## FxA

### What's New

- We are now using [Protocol Buffers](https://developers.google.com/protocol-buffers/) to pass the Profile data across the FFI boundaries, both on Android and iOS. On Android there should be no breaking changes.

### Breaking changes

- iOS: You now have to include the `SwiftProtobuf` framework in your projects for FxAClient to work (otherwise you'll get a runtime error when fetching the user profile). It is built into `Carthage/Build/iOS` just like `FxAClient.framework`.
- iOS: In order to build FxAClient from source, you need [swift-protobuf](https://github.com/apple/swift-protobuf) installed. Simply run `brew install swift-protobuf` if you have Homebrew.
- iOS: You need to run `carthage bootstrap` at the root of the repository at least once before building the FxAClient project: this will build the `SwiftProtobuf.framework` file needed by the project.
- iOS: the `Profile` class now inherits from `RustProtobuf`. Nothing should change in practice for you.

# 0.16.1 (_2019-02-08_)

[Full Changelog](https://github.com/mozilla/application-services/compare/v0.16.0...v0.16.1)
Expand Down
125 changes: 117 additions & 8 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cartfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
github "apple/swift-protobuf" ~> 1.0
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ buildscript {
// Downloading libs/ archives from Taskcluster.
classpath 'de.undercouch:gradle-download-task:3.4.3'

classpath 'com.google.protobuf:protobuf-gradle-plugin:0.8.8'
eoger marked this conversation as resolved.
Show resolved Hide resolved

// NOTE: Do not place your application dependencies here; they belong
// in the individual module build.gradle files
}
Expand Down
6 changes: 6 additions & 0 deletions components/fxa-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ authors = ["Edouard Oger <eoger@fastmail.com>"]
[dependencies]
base64 = "0.9.3"
byteorder = "1.2.6"
bytes = "0.4"
eoger marked this conversation as resolved.
Show resolved Hide resolved
failure = "0.1.3"
hawk = { version = "1.0.5", optional = true }
hex = "0.3.2"
lazy_static = "1.0.0"
log = "0.4"
openssl = { version = "0.10.12", optional = true }
prost = "0.4"
prost-derive = "0.4"
regex = "1.0.0"
reqwest = "0.9.7"
ring = "0.14.5"
Expand All @@ -26,6 +29,9 @@ ffi-support = { path = "../support/ffi", optional = true }
[dev-dependencies]
text_io = "0.1.7"

[build-dependencies]
prost-build = "0.4"

[features]
browserid = ["openssl", "hawk"]
ffi = ["ffi-support"]
Expand Down
31 changes: 31 additions & 0 deletions components/fxa-client/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apply plugin: 'com.android.library'
apply plugin: 'org.mozilla.rust-android-gradle.rust-android'
apply plugin: 'kotlin-android'
apply plugin: 'kotlin-android-extensions'
apply plugin: 'com.google.protobuf'

android {
compileSdkVersion 27
Expand Down Expand Up @@ -31,6 +32,14 @@ android {
packagingOptions {
doNotStrip "**/*.so"
}

sourceSets {
main {
proto {
srcDir '../src'
eoger marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

configurations {
Expand Down Expand Up @@ -71,9 +80,31 @@ cargo {
exec = rootProject.ext.cargoExec
}

protobuf {
protoc {
artifact = 'com.google.protobuf:protoc:3.0.0'
}
plugins {
javalite {
artifact = 'com.google.protobuf:protoc-gen-javalite:3.0.0'
}
}
generateProtoTasks {
eoger marked this conversation as resolved.
Show resolved Hide resolved
all().each { task ->
task.builtins {
remove java
}
task.plugins {
javalite { }
}
}
}
}

dependencies {
implementation 'net.java.dev.jna:jna:4.5.2@aar'
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
implementation 'com.google.protobuf:protobuf-lite:3.0.0'
}

afterEvaluate {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package org.mozilla.fxaclient.internal

import com.google.protobuf.CodedInputStream
import com.sun.jna.Pointer
import com.sun.jna.Structure
import java.util.*

internal open class ByteBuffer : Structure() {
eoger marked this conversation as resolved.
Show resolved Hide resolved
@JvmField var len: Long = 0
@JvmField var data: Pointer? = null

init {
read()
}

override fun getFieldOrder(): List<String> {
return Arrays.asList("len", "data")
}

fun asCodedInputStream(): CodedInputStream? {
return this.data?.let {
CodedInputStream.newInstance(it.getByteBuffer(0, this.len))
}
}

class ByValue : ByteBuffer(), Structure.ByValue
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ internal interface FxaClient : Library {
e: Error.ByReference
): Pointer?

fun fxa_profile(fxa: FxaHandle, ignoreCache: Boolean, e: Error.ByReference): Profile.Raw?
fun fxa_profile(fxa: FxaHandle, ignoreCache: Boolean, e: Error.ByReference): ByteBuffer.ByValue

fun fxa_get_token_server_endpoint_url(fxa: FxaHandle, e: Error.ByReference): Pointer?
fun fxa_get_connection_success_url(fxa: FxaHandle, e: Error.ByReference): Pointer?
Expand All @@ -89,7 +89,7 @@ internal interface FxaClient : Library {
// when using Structure.
fun fxa_oauth_info_free(ptr: Pointer)

fun fxa_profile_free(ptr: Pointer)
fun fxa_bytebuffer_free(buffer: ByteBuffer.ByValue)
}
internal typealias FxaHandle = Long

Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,32 @@

package org.mozilla.fxaclient.internal

import com.sun.jna.Pointer
import com.sun.jna.Structure
import ffi_types.FfiTypes.Profile as RawProfile

import java.util.Arrays

class Profile internal constructor(raw: Raw) {
class Profile internal constructor(byteBuffer: ByteBuffer.ByValue) {

val uid: String?
val email: String?
val avatar: String?
val avatarDefault: Boolean
val displayName: String?

internal class Raw(p: Pointer) : Structure(p) {
@JvmField var uid: Pointer? = null
@JvmField var email: Pointer? = null
@JvmField var avatar: Pointer? = null
@JvmField var avatarDefault: Byte = 0
@JvmField var displayName: Pointer? = null

init {
read()
}

override fun getFieldOrder(): List<String> {
return Arrays.asList("uid", "email", "avatar", "avatarDefault", "displayName")
}
}

init {
try {
this.uid = raw.uid?.getRustString()
this.email = raw.email?.getRustString()
this.avatar = raw.avatar?.getRustString()
this.avatarDefault = raw.avatarDefault == 1.toByte()
this.displayName = raw.displayName?.getRustString()
val raw = byteBuffer.asCodedInputStream()?.let {
RawProfile.parseFrom(it)
} ?: run {
// TODO: should throw somehow?
eoger marked this conversation as resolved.
Show resolved Hide resolved
RawProfile.getDefaultInstance()
}

this.uid = if (raw.hasUid()) raw.uid else null
this.email = if (raw.hasEmail()) raw.email else null
this.avatar = if (raw.hasAvatar()) raw.avatar else null
this.avatarDefault = raw.avatarDefault
this.displayName = if (raw.hasDisplayName()) raw.displayName else null
} finally {
FxaClient.INSTANCE.fxa_profile_free(raw.pointer)
FxaClient.INSTANCE.fxa_bytebuffer_free(byteBuffer)
}
}
}
7 changes: 7 additions & 0 deletions components/fxa-client/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

fn main() {
prost_build::compile_protos(&["src/ffi_types.proto"], &["src/"]).unwrap();
eoger marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 1 addition & 1 deletion components/fxa-client/ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ name = "fxaclient_ffi"
crate-type = ["lib", "staticlib", "cdylib"]

[dependencies]
ffi-support = { path = "../../support/ffi" }
ffi-support = { path = "../../support/ffi", features = ["prost_support"] }
log = "0.4.6"
lazy_static = "1.2.0"

Expand Down
15 changes: 9 additions & 6 deletions components/fxa-client/ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use ffi_support::{
define_box_destructor, define_handle_map_deleter, define_string_destructor, rust_str_from_c,
ConcurrentHandleMap, ExternError,
define_box_destructor, define_bytebuffer_destructor, define_handle_map_deleter,
define_string_destructor, rust_str_from_c, ByteBuffer, ConcurrentHandleMap, ExternError,
};
use fxa_client::{ffi::*, FirefoxAccount, PersistCallback};
use fxa_client::{ffi::*, ffi_types, FirefoxAccount, PersistCallback};
use std::{ffi::CString, os::raw::c_char};

#[no_mangle]
Expand Down Expand Up @@ -147,9 +147,12 @@ pub extern "C" fn fxa_profile(
handle: u64,
ignore_cache: bool,
error: &mut ExternError,
) -> *mut ProfileC {
) -> ByteBuffer {
eoger marked this conversation as resolved.
Show resolved Hide resolved
log::debug!("fxa_profile");
ACCOUNTS.call_with_result_mut(error, handle, |fxa| fxa.get_profile(ignore_cache))
ACCOUNTS.call_with_result_mut(error, handle, |fxa| {
fxa.get_profile(ignore_cache)
.map(|p| Into::<ffi_types::Profile>::into(p))
})
}

/// Get the Sync token server endpoint URL.
Expand Down Expand Up @@ -325,6 +328,6 @@ define_string_destructor!(fxa_str_free);

define_handle_map_deleter!(ACCOUNTS, fxa_free);
define_box_destructor!(AccessTokenInfoC, fxa_oauth_info_free);
define_box_destructor!(ProfileC, fxa_profile_free);
#[cfg(feature = "browserid")]
define_box_destructor!(SyncKeysC, fxa_sync_keys_free);
define_bytebuffer_destructor!(fxa_bytebuffer_free);
27 changes: 27 additions & 0 deletions components/fxa-client/ios/FxAClient.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@
objects = {

/* Begin PBXBuildFile section */
CE2F4D42220E0014001ECF92 /* ffi_types.proto in Sources */ = {isa = PBXBuildFile; fileRef = CE2F4D41220E0014001ECF92 /* ffi_types.proto */; };
CE3E9D3520A36B63001B4B14 /* libfxaclient_ffi.a in Frameworks */ = {isa = PBXBuildFile; fileRef = CE7B4B8A20A36B0500FC4422 /* libfxaclient_ffi.a */; };
CE9D202520914D0D00F1C8FA /* FxAClient.h in Headers */ = {isa = PBXBuildFile; fileRef = CE9D202320914D0D00F1C8FA /* FxAClient.h */; settings = {ATTRIBUTES = (Public, ); }; };
CE9D203120914D2600F1C8FA /* FirefoxAccount.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE9D202B20914D2600F1C8FA /* FirefoxAccount.swift */; };
CE9D203520914D2600F1C8FA /* fxa.h in Headers */ = {isa = PBXBuildFile; fileRef = CE9D202F20914D2600F1C8FA /* fxa.h */; settings = {ATTRIBUTES = (Public, ); }; };
CE5A6AEF220E2FD300B7F1BC /* RustProtobuf.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE5A6AEE220E2FD300B7F1BC /* RustProtobuf.swift */; };
CEBACFC4220E22C80078D41C /* SwiftProtobuf.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CEBACFC3220E22C80078D41C /* SwiftProtobuf.framework */; };
CECB395D20B5BE0200DB3ED4 /* RustPointer.swift in Sources */ = {isa = PBXBuildFile; fileRef = CECB395B20B5BE0200DB3ED4 /* RustPointer.swift */; };
CEE1087620C5ADF9007048AC /* FxAError.swift in Sources */ = {isa = PBXBuildFile; fileRef = CEE1087520C5ADF9007048AC /* FxAError.swift */; };
EBA8770C21F5FD5D004F63F0 /* base.xcconfig in Resources */ = {isa = PBXBuildFile; fileRef = EBA8770921F5FD5D004F63F0 /* base.xcconfig */; };
Expand All @@ -21,13 +24,30 @@
EBE26B4B220B4DE300D1D99A /* SerialQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = EBE26B4A220B4DE300D1D99A /* SerialQueue.swift */; };
/* End PBXBuildFile section */

/* Begin PBXBuildRule section */
CE2F4D40220DFF7B001ECF92 /* PBXBuildRule */ = {
isa = PBXBuildRule;
compilerSpec = com.apple.compilers.proxy.script;
filePatterns = "*.proto";
fileType = pattern.proxy;
isEditable = 1;
outputFiles = (
"$(DERIVED_FILE_DIR)/$(INPUT_FILE_BASE).pb.swift",
);
script = "protoc --proto_path=$INPUT_FILE_DIR --swift_out=$DERIVED_FILE_DIR $INPUT_FILE_PATH\n";
};
/* End PBXBuildRule section */

/* Begin PBXFileReference section */
CE2F4D41220E0014001ECF92 /* ffi_types.proto */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.protobuf; name = ffi_types.proto; path = ../src/ffi_types.proto; sourceTree = "<group>"; };
CE5A6AEE220E2FD300B7F1BC /* RustProtobuf.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RustProtobuf.swift; sourceTree = "<group>"; };
CE7B4B8A20A36B0500FC4422 /* libfxaclient_ffi.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libfxaclient_ffi.a; path = ../../../target/universal/debug/libfxaclient_ffi.a; sourceTree = "<group>"; };
CE9D202020914D0D00F1C8FA /* FxAClient.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = FxAClient.framework; sourceTree = BUILT_PRODUCTS_DIR; };
CE9D202320914D0D00F1C8FA /* FxAClient.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = FxAClient.h; sourceTree = "<group>"; };
CE9D202420914D0D00F1C8FA /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
CE9D202B20914D2600F1C8FA /* FirefoxAccount.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FirefoxAccount.swift; sourceTree = "<group>"; };
CE9D202F20914D2600F1C8FA /* fxa.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = fxa.h; sourceTree = "<group>"; };
CEBACFC3220E22C80078D41C /* SwiftProtobuf.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = SwiftProtobuf.framework; path = ../../../Carthage/Build/iOS/SwiftProtobuf.framework; sourceTree = "<group>"; };
CECB395B20B5BE0200DB3ED4 /* RustPointer.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RustPointer.swift; sourceTree = "<group>"; };
CEE1087520C5ADF9007048AC /* FxAError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FxAError.swift; sourceTree = "<group>"; };
EBA8770921F5FD5D004F63F0 /* base.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = base.xcconfig; sourceTree = "<group>"; };
Expand All @@ -43,6 +63,7 @@
isa = PBXFrameworksBuildPhase;
buildActionMask = 2147483647;
files = (
CEBACFC4220E22C80078D41C /* SwiftProtobuf.framework in Frameworks */,
CE3E9D3520A36B63001B4B14 /* libfxaclient_ffi.a in Frameworks */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand All @@ -53,6 +74,7 @@
CE9D201620914D0D00F1C8FA = {
isa = PBXGroup;
children = (
CE2F4D41220E0014001ECF92 /* ffi_types.proto */,
EBA8770921F5FD5D004F63F0 /* base.xcconfig */,
EBA8770A21F5FD5D004F63F0 /* debug.xcconfig */,
EBA8770B21F5FD5D004F63F0 /* release.xcconfig */,
Expand Down Expand Up @@ -88,6 +110,7 @@
CE9D203720914D4800F1C8FA /* Frameworks */ = {
isa = PBXGroup;
children = (
CEBACFC3220E22C80078D41C /* SwiftProtobuf.framework */,
CE7B4B8A20A36B0500FC4422 /* libfxaclient_ffi.a */,
);
name = Frameworks;
Expand All @@ -114,6 +137,7 @@
isa = PBXGroup;
children = (
CECB395B20B5BE0200DB3ED4 /* RustPointer.swift */,
CE5A6AEE220E2FD300B7F1BC /* RustProtobuf.swift */,
);
path = Rust;
sourceTree = "<group>";
Expand Down Expand Up @@ -144,6 +168,7 @@
CE9D201E20914D0D00F1C8FA /* Resources */,
);
buildRules = (
CE2F4D40220DFF7B001ECF92 /* PBXBuildRule */,
);
dependencies = (
);
Expand Down Expand Up @@ -219,9 +244,11 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
CE2F4D42220E0014001ECF92 /* ffi_types.proto in Sources */,
CECB395D20B5BE0200DB3ED4 /* RustPointer.swift in Sources */,
EBE26B4B220B4DE300D1D99A /* SerialQueue.swift in Sources */,
EBE26B42220B3DF700D1D99A /* String+Free_FxAClient.swift in Sources */,
CE5A6AEF220E2FD300B7F1BC /* RustProtobuf.swift in Sources */,
EBE26B49220B4D0200D1D99A /* CommonErrors.swift in Sources */,
CE9D203120914D2600F1C8FA /* FirefoxAccount.swift in Sources */,
CEE1087620C5ADF9007048AC /* FxAError.swift in Sources */,
Expand Down
Loading