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.

Sign up for GitHub

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

Jump to bottom

feat(dist): Add limit validation #1386

Merged
merged 4 commits into from
Nov 7, 2022
Merged

feat(dist): Add limit validation #1386

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
34f6c4a
feat(dist): Add limit validation
olksdr Nov 4, 2022
f1b22dc
Merge branch 'master' into feat/add-limit-varification-to-distribution
olksdr Nov 7, 2022
b7da49b
feat(dist): handle unicode properly for validation of number of chars
olksdr Nov 7, 2022
b5d7a8a
s/value_parser/validator/g
kamilogorek Nov 7, 2022
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
2 changes: 2 additions & 0 deletions src/commands/files/upload.rs
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ use symbolic::debuginfo::sourcebundle::SourceFileType;

use crate::api::{Api, ProgressBarMode};
use crate::config::Config;
use crate::utils::args::validate_distribution;
use crate::utils::file_search::ReleaseFileSearch;
use crate::utils::file_upload::{ReleaseFile, ReleaseFileUpload, UploadContext};
use crate::utils::fs::{decompress_gzip_content, is_gzip_compressed, path_as_url};
Expand All @@ -35,6 +36,7 @@ pub fn make_command(command: Command) -> Command {
.long("dist")
.short('d')
.value_name("DISTRIBUTION")
.value_parser(validate_distribution)
.help("Optional distribution identifier for this file."),
)
.arg(
Expand Down
3 changes: 2 additions & 1 deletion src/commands/react_native/appcenter.rs
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ use log::info;
use crate::api::{Api, NewRelease};
use crate::config::Config;
use crate::utils::appcenter::{get_appcenter_package, get_react_native_appcenter_release};
use crate::utils::args::ArgExt;
use crate::utils::args::{validate_distribution, ArgExt};
use crate::utils::file_search::ReleaseFileSearch;
use crate::utils::file_upload::UploadContext;
use crate::utils::sourcemaps::SourceMapProcessor;
Expand Down Expand Up @@ -50,6 +50,7 @@ pub fn make_command(command: Command) -> Command {
.long("dist")
.value_name("DISTRIBUTION")
.multiple_occurrences(true)
.value_parser(validate_distribution)
.help("The names of the distributions to publish. Can be supplied multiple times."),
)
.arg(
Expand Down
3 changes: 2 additions & 1 deletion src/commands/react_native/gradle.rs
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@ use sourcemap::ram_bundle::RamBundle;

use crate::api::{Api, NewRelease};
use crate::config::Config;
use crate::utils::args::ArgExt;
use crate::utils::args::{validate_distribution, ArgExt};
use crate::utils::file_search::ReleaseFileSearch;
use crate::utils::file_upload::UploadContext;
use crate::utils::sourcemaps::SourceMapProcessor;
Expand Down Expand Up @@ -45,6 +45,7 @@ pub fn make_command(command: Command) -> Command {
.value_name("DISTRIBUTION")
.required(true)
.multiple_occurrences(true)
.value_parser(validate_distribution)
.help("The names of the distributions to publish. Can be supplied multiple times."),
)
.arg(
Expand Down
3 changes: 2 additions & 1 deletion src/commands/react_native/xcode.rs
View file
Open in desktop
Expand Up @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize};

use crate::api::{Api, NewRelease};
use crate::config::Config;
use crate::utils::args::ArgExt;
use crate::utils::args::{validate_distribution, ArgExt};
use crate::utils::file_search::ReleaseFileSearch;
use crate::utils::file_upload::UploadContext;
use crate::utils::fs::TempFile;
Expand Down Expand Up @@ -69,6 +69,7 @@ pub fn make_command(command: Command) -> Command {
.long("dist")
.value_name("DISTRIBUTION")
.multiple_occurrences(true)
.value_parser(validate_distribution)
.help("The names of the distributions to publish. Can be supplied multiple times."),
)
.arg(
Expand Down
3 changes: 2 additions & 1 deletion src/commands/send_event.rs
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@ use serde_json::Value;
use username::get_user_name;

use crate::config::Config;
use crate::utils::args::{get_timestamp, validate_timestamp};
use crate::utils::args::{get_timestamp, validate_distribution, validate_timestamp};
use crate::utils::event::{attach_logfile, get_sdk_info, with_sentry_client};
use crate::utils::releases::detect_release_name;

Expand Down Expand Up @@ -58,6 +58,7 @@ pub fn make_command(command: Command) -> Command {
.value_name("DISTRIBUTION")
.long("dist")
.short('d')
.value_parser(validate_distribution)
.help("Set the distribution."),
)
.arg(
Expand Down
2 changes: 2 additions & 0 deletions src/commands/sourcemaps/upload.rs
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@ use sha1_smol::Digest;

use crate::api::{Api, NewRelease};
use crate::config::Config;
use crate::utils::args::validate_distribution;
use crate::utils::file_search::ReleaseFileSearch;
use crate::utils::file_upload::UploadContext;
use crate::utils::fs::path_as_url;
Expand Down Expand Up @@ -44,6 +45,7 @@ pub fn make_command(command: Command) -> Command {
.long("dist")
.short('d')
.value_name("DISTRIBUTION")
.value_parser(validate_distribution)
.help("Optional distribution identifier for the sourcemaps."),
)
.arg(
Expand Down
16 changes: 16 additions & 0 deletions src/utils/args.rs
View file
Open in desktop
Expand Up @@ -50,6 +50,22 @@ fn validate_release(v: &str) -> Result<(), String> {
}
}

pub fn validate_distribution(v: &str) -> Result<(), String> {
if v.trim() != v {
Err(
"Invalid distribution name. Distribution must not contain leading or trailing spaces."
.to_string(),
)
} else if v.len() > 64 {
Err(
"Invalid distribution name. Distribution name must not be longer than 64 characters."
.to_string(),
Copy link
Member

Choose a reason for hiding this comment

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

This gets me thinking, how do we handle unicode characters? They might exceed string length 64 even if the number of characters is lower. I assume that the limit enforced on sentry side is applied to the number of characters, not the number of bytes.

Copy link
Contributor Author

@olksdr olksdr Nov 4, 2022 •
edited

Choose a reason for hiding this comment

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

Very good point! On the relay side we use bytecount::num_chars and check the actual bytes. We might want to do here similar.

)
} else {
Ok(())
}
}

pub fn validate_int(v: &str) -> Result<(), String> {
if v.parse::<i64>().is_ok() {
Ok(())
Expand Down
21 changes: 21 additions & 0 deletions tests/integration/_cases/send_event/send_event-raw-fail.trycmd
View file
Open in desktop
@@ -0,0 +1,21 @@
```
$ sentry-cli send-event --log-level=debug
> --level debug
> --timestamp 1649335000929
> --release my-release
> --dist 11111111111111111111111111111111111111111111111111111111111111111
> --env production
> --message hello
> --platform prod
> --tag "hello:there"
> --extra "hello:there"
> --user "id:42"
> --fingerprint custom-fingerprint
> --no-environ
? 2
error: Invalid value "11111111111111111111111111111111111111111111111111111111111111111" for '--dist <DISTRIBUTION>': Invalid distribution name. Distribution name must not be longer than 64 characters.

For more information try --help

```

5 changes: 5 additions & 0 deletions tests/integration/send_event.rs
View file
Open in desktop
Expand Up @@ -18,3 +18,8 @@ fn command_send_event_raw() {
fn command_send_event_file() {
register_test("send_event/send_event-file.trycmd");
}

#[test]
fn command_send_event_raw_fail() {
register_test("send_event/send_event-raw-fail.trycmd");
}