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

feat(protocol): Add valid platforms as constant #294

Merged
merged 1 commit into from
Oct 31, 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
1 change: 1 addition & 0 deletions cabi/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion cabi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ lto = true
[dependencies]
chrono = "0.4.7"
failure = "0.1.5"
json-forensics = { version = "*", git = "https://github.com/getsentry/rust-json-forensics" }
lazy_static = "1.3.0"
serde = {version = "1.0.98", features = ["derive"]}
serde_json = "1.0.40"
uuid = "0.7.4"
json-forensics = { version = "*", git = "https://github.com/getsentry/rust-json-forensics" }

[dependencies.semaphore-common]
path = "../common"
Expand Down
5 changes: 5 additions & 0 deletions cabi/include/semaphore.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ bool semaphore_uuid_is_nil(const SemaphoreUuid *uuid);
*/
SemaphoreStr semaphore_uuid_to_str(const SemaphoreUuid *uuid);

/**
* Returns a list of all valid platform identifiers.
*/
const SemaphoreStr *semaphore_valid_platforms(uintptr_t *size_out);

/**
* Validates a register response.
*/
Expand Down
5 changes: 5 additions & 0 deletions cabi/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ impl SemaphoreErrorCode {
}
}

// SemaphoreStr is immutable, thus it can be Send + Sync

unsafe impl Sync for SemaphoreStr {}
unsafe impl Send for SemaphoreStr {}

impl Default for SemaphoreStr {
fn default() -> SemaphoreStr {
SemaphoreStr {
Expand Down
20 changes: 19 additions & 1 deletion cabi/src/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use semaphore_common::{glob_match_bytes, GlobOptions};
use semaphore_general::datascrubbing::DataScrubbingConfig;
use semaphore_general::pii::PiiProcessor;
use semaphore_general::processor::{process_value, split_chunks, ProcessingState};
use semaphore_general::protocol::Event;
use semaphore_general::protocol::{Event, VALID_PLATFORMS};
use semaphore_general::store::{GeoIpLookup, StoreConfig, StoreProcessor};
use semaphore_general::types::{Annotated, Remark};

Expand All @@ -20,6 +20,11 @@ use crate::core::{SemaphoreBuf, SemaphoreStr};
pub struct SemaphoreGeoIpLookup;
pub struct SemaphoreStoreNormalizer;

lazy_static::lazy_static! {
static ref VALID_PLATFORM_STRS: Vec<SemaphoreStr> =
VALID_PLATFORMS.iter().map(|s| SemaphoreStr::new(s)).collect();
}

ffi_fn! {
unsafe fn semaphore_split_chunks(
string: *const SemaphoreStr,
Expand Down Expand Up @@ -53,6 +58,19 @@ ffi_fn! {
}
}

ffi_fn! {
/// Returns a list of all valid platform identifiers.
unsafe fn semaphore_valid_platforms(
size_out: *mut usize,
) -> Result<*const SemaphoreStr> {
if let Some(size_out) = size_out.as_mut() {
*size_out = VALID_PLATFORM_STRS.len();
}

Ok(VALID_PLATFORM_STRS.as_ptr())
}
}

ffi_fn! {
unsafe fn semaphore_store_normalizer_new(
config: *const SemaphoreStr,
Expand Down
21 changes: 21 additions & 0 deletions general/src/protocol/constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
pub const VALID_PLATFORMS: &[&str] = &[
"as3",
"c",
"cfml",
"cocoa",
"csharp",
"elixir",
"go",
"groovy",
"haskell",
"java",
"javascript",
"native",
"node",
"objc",
"other",
"perl",
"php",
"python",
"ruby",
];
2 changes: 2 additions & 0 deletions general/src/protocol/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Implements the sentry event protocol.
mod breadcrumb;
mod clientsdk;
mod constants;
mod contexts;
mod debugmeta;
mod event;
Expand All @@ -20,6 +21,7 @@ mod user;

pub use self::breadcrumb::Breadcrumb;
pub use self::clientsdk::{ClientSdkInfo, ClientSdkPackage};
pub use self::constants::VALID_PLATFORMS;
pub use self::contexts::{
AppContext, BrowserContext, Context, ContextInner, Contexts, DeviceContext, OperationType,
OsContext, RuntimeContext, SpanId, TraceContext, TraceId,
Expand Down
2 changes: 0 additions & 2 deletions general/src/store/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//! Utility code for sentry's internal store.
use std::collections::BTreeSet;
use std::sync::Arc;

use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -31,7 +30,6 @@ pub struct StoreConfig {
pub protocol_version: Option<String>,
pub grouping_config: Option<Value>,

pub valid_platforms: BTreeSet<String>, // TODO(ja): Pending removal
pub max_secs_in_future: Option<i64>,
pub max_secs_in_past: Option<i64>,
pub enable_trimming: Option<bool>,
Expand Down
18 changes: 8 additions & 10 deletions general/src/store/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use uuid::Uuid;
use crate::processor::{MaxChars, ProcessValue, ProcessingState, Processor};
use crate::protocol::{
AsPair, Breadcrumb, ClientSdkInfo, Context, DebugImage, Event, EventId, EventType, Exception,
Frame, IpAddr, Level, LogEntry, Request, Stacktrace, Tags, User,
Frame, IpAddr, Level, LogEntry, Request, Stacktrace, Tags, User, VALID_PLATFORMS,
};
use crate::store::{GeoIpLookup, StoreConfig};
use crate::types::{
Expand Down Expand Up @@ -60,6 +60,10 @@ impl DedupCache {
}
}

pub fn is_valid_platform(platform: &str) -> bool {
VALID_PLATFORMS.contains(&platform)
}

/// The processor that normalizes events for store.
pub struct NormalizeProcessor<'a> {
config: Arc<StoreConfig>,
Expand Down Expand Up @@ -367,7 +371,7 @@ impl<'a> Processor for NormalizeProcessor<'a> {

// Validate basic attributes
event.platform.apply(|platform, _| {
if self.config.valid_platforms.contains(platform) {
if is_valid_platform(&platform) {
Ok(())
} else {
Err(ProcessingAction::DeleteValueSoft)
Expand Down Expand Up @@ -698,9 +702,7 @@ fn test_user_ip_from_remote_addr() {
..Event::default()
});

let mut config = StoreConfig::default();
config.valid_platforms.insert("javascript".to_owned());

let config = StoreConfig::default();
let mut processor = NormalizeProcessor::new(Arc::new(config), None);
process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();

Expand Down Expand Up @@ -734,9 +736,7 @@ fn test_user_ip_from_invalid_remote_addr() {
..Event::default()
});

let mut config = StoreConfig::default();
config.valid_platforms.insert("javascript".to_owned());

let config = StoreConfig::default();
let mut processor = NormalizeProcessor::new(Arc::new(config), None);
process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();

Expand All @@ -752,7 +752,6 @@ fn test_user_ip_from_client_ip_without_auto() {

let mut config = StoreConfig::default();
config.client_ip = Some(IpAddr::parse("213.47.147.207").unwrap());
config.valid_platforms.insert("javascript".to_owned());

let mut processor = NormalizeProcessor::new(Arc::new(config), None);
process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();
Expand Down Expand Up @@ -781,7 +780,6 @@ fn test_user_ip_from_client_ip_with_auto() {

let mut config = StoreConfig::default();
config.client_ip = Some(IpAddr::parse("213.47.147.207").unwrap());
config.valid_platforms.insert("javascript".to_owned());

let geo = GeoIpLookup::open(concat!(
env!("CARGO_MANIFEST_DIR"),
Expand Down
30 changes: 1 addition & 29 deletions general/tests/test_fixtures.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::collections::BTreeSet;
use std::fs;

use insta::assert_yaml_snapshot;
Expand All @@ -7,41 +6,14 @@ use semaphore_general::protocol::Event;
use semaphore_general::store::{StoreConfig, StoreProcessor};
use semaphore_general::types::{Annotated, SerializableAnnotated};

lazy_static::lazy_static! {
static ref VALID_PLATOFORMS: BTreeSet<String> = {
let mut platforms = BTreeSet::new();
platforms.insert("as3".to_string());
platforms.insert("c".to_string());
platforms.insert("cfml".to_string());
platforms.insert("cocoa".to_string());
platforms.insert("csharp".to_string());
platforms.insert("go".to_string());
platforms.insert("java".to_string());
platforms.insert("javascript".to_string());
platforms.insert("node".to_string());
platforms.insert("objc".to_string());
platforms.insert("other".to_string());
platforms.insert("perl".to_string());
platforms.insert("php".to_string());
platforms.insert("python".to_string());
platforms.insert("ruby".to_string());
platforms.insert("elixir".to_string());
platforms.insert("haskell".to_string());
platforms.insert("groovy".to_string());
platforms.insert("native".to_string());
platforms
};
}

macro_rules! event_snapshot {
($id:expr) => {
let data = fs::read_to_string(format!("tests/fixtures/payloads/{}.json", $id)).unwrap();

let mut event = Annotated::<Event>::from_json(&data).unwrap();
assert_yaml_snapshot!(SerializableAnnotated(&event));

let mut config = StoreConfig::default();
config.valid_platforms = VALID_PLATOFORMS.clone();
let config = StoreConfig::default();
let mut processor = StoreProcessor::new(config, None);
process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();
assert_yaml_snapshot!(SerializableAnnotated(&event), {
Expand Down
20 changes: 20 additions & 0 deletions py/semaphore/processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,29 @@
"GeoIpLookup",
"scrub_event",
"is_glob_match",
"VALID_PLATFORMS",
]


VALID_PLATFORMS = frozenset()


def _init_valid_platforms():
global VALID_PLATFORMS

size_out = ffi.new("uintptr_t *")
strings = rustcall(lib.semaphore_valid_platforms, size_out)

valid_platforms = []
for i in range(int(size_out[0])):
valid_platforms.append(decode_str(strings[i]))

VALID_PLATFORMS = frozenset(valid_platforms)


_init_valid_platforms()


def split_chunks(string, remarks):
return json.loads(
decode_str(
Expand Down
5 changes: 5 additions & 0 deletions py/tests/test_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
}


def test_valid_platforms():
assert len(semaphore.VALID_PLATFORMS) > 0
assert "native" in semaphore.VALID_PLATFORMS


def test_split_chunks():
chunks = semaphore.split_chunks(TEXT, REMARKS)
assert chunks == CHUNKS
Expand Down
1 change: 0 additions & 1 deletion server/src/actors/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ impl EventProcessor {
key_id,
protocol_version: Some(auth.version().to_string()),
grouping_config: message.project_state.config.grouping_config.clone(),
valid_platforms: Default::default(), // TODO(ja): Pending removal
max_secs_in_future: Some(self.config.max_secs_in_future()),
max_secs_in_past: Some(self.config.max_secs_in_past()),
enable_trimming: Some(true),
Expand Down